Added new DriveNotificationObserver interface which all observers of Google Drive invalidations sho…

Added new DriveNotificationObserver interface which all observers of Google Drive invalidations should implement.

BUG=173339
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193356

[calvinlo, 2013-04-09 08:05:48]

Taiju san, can you please take a look.

[tzik, 2013-04-09 08:31:34]

https://codereview.chromium.org/13940004/diff/1/chrome/browser/google_apis/dr...
File chrome/browser/google_apis/drive_notification_observer.h (right):

https://codereview.chromium.org/13940004/diff/1/chrome/browser/google_apis/dr...
chrome/browser/google_apis/drive_notification_observer.h:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
s/2012/2013/

https://codereview.chromium.org/13940004/diff/1/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/drive_file_sync_service.h (right):

https://codereview.chromium.org/13940004/diff/1/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/drive_file_sync_service.h:126: virtual void
CheckForUpdates();
Please add OVERRIDE.

[calvinlo, 2013-04-09 08:44:20]

https://codereview.chromium.org/13940004/diff/1/chrome/browser/google_apis/dr...
File chrome/browser/google_apis/drive_notification_observer.h (right):

https://codereview.chromium.org/13940004/diff/1/chrome/browser/google_apis/dr...
chrome/browser/google_apis/drive_notification_observer.h:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
On 2013/04/09 08:31:34, tzik wrote:
> s/2012/2013/

Done.

https://codereview.chromium.org/13940004/diff/1/chrome/browser/sync_file_syst...
File chrome/browser/sync_file_system/drive_file_sync_service.h (right):

https://codereview.chromium.org/13940004/diff/1/chrome/browser/sync_file_syst...
chrome/browser/sync_file_system/drive_file_sync_service.h:126: virtual void
CheckForUpdates();
On 2013/04/09 08:31:34, tzik wrote:
> Please add OVERRIDE.

Done.

[tzik, 2013-04-09 08:52:30]

lgtm

https://codereview.chromium.org/13940004/diff/3006/chrome/browser/google_apis...
File chrome/browser/google_apis/drive_notification_observer.h (right):

https://codereview.chromium.org/13940004/diff/3006/chrome/browser/google_apis...
chrome/browser/google_apis/drive_notification_observer.h:7: 
#include "base/basictypes.h" ?

[calvinlo, 2013-04-09 08:58:39]

https://codereview.chromium.org/13940004/diff/3006/chrome/browser/google_apis...
File chrome/browser/google_apis/drive_notification_observer.h (right):

https://codereview.chromium.org/13940004/diff/3006/chrome/browser/google_apis...
chrome/browser/google_apis/drive_notification_observer.h:7: 
On 2013/04/09 08:52:30, tzik wrote:
> #include "base/basictypes.h" ?

Done.

[calvinlo, 2013-04-09 09:18:35]

+thestig for chrome_browser.gypi review

[calvinlo, 2013-04-09 09:19:34]

+satorux for google_apis/ ownership review. 
Hi Satoru, can you please review the file I added or reassign to someone on the
ChromeOS team to review. Thanks!

[tfarina, 2013-04-09 13:30:13]

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
File chrome/browser/google_apis/drive_notification_observer.h (right):

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:8: #include
"base/basictypes.h"
remove this if you remove the macro below.

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:16:
DriveNotificationObserver() {}
no constructor.

you can't instantiate this class without subclassing it.

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:17: virtual
~DriveNotificationObserver() {}
can you make this protected?

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:23:
DISALLOW_COPY_AND_ASSIGN(DriveNotificationObserver);
nor this.

because you have a pure abstract interface. Yeah, I know this disallow the copy
and assign the subclass, but it's subtle.

[Lei Zhang, 2013-04-09 19:16:22]

.gypi file lgtm, but please address tfarina's comments.

[calvinlo, 2013-04-10 04:05:42]

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
File chrome/browser/google_apis/drive_notification_observer.h (right):

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:8: #include
"base/basictypes.h"
On 2013/04/09 13:30:13, tfarina wrote:
> remove this if you remove the macro below.

Done.

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:16:
DriveNotificationObserver() {}
On 2013/04/09 13:30:13, tfarina wrote:
> no constructor.
> 
> you can't instantiate this class without subclassing it.

Done.

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:17: virtual
~DriveNotificationObserver() {}
On 2013/04/09 13:30:13, tfarina wrote:
> can you make this protected?

Done.

https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
chrome/browser/google_apis/drive_notification_observer.h:23:
DISALLOW_COPY_AND_ASSIGN(DriveNotificationObserver);
On 2013/04/09 13:30:13, tfarina wrote:
> nor this.
> 
> because you have a pure abstract interface. Yeah, I know this disallow the
copy
> and assign the subclass, but it's subtle.

Done.

[kinaba, 2013-04-10 04:09:16]

On 2013/04/10 04:05:42, calvinlo wrote:
>
https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
> File chrome/browser/google_apis/drive_notification_observer.h (right):
> 
>
https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
> chrome/browser/google_apis/drive_notification_observer.h:8: #include
> "base/basictypes.h"
> On 2013/04/09 13:30:13, tfarina wrote:
> > remove this if you remove the macro below.
> 
> Done.
> 
>
https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
> chrome/browser/google_apis/drive_notification_observer.h:16:
> DriveNotificationObserver() {}
> On 2013/04/09 13:30:13, tfarina wrote:
> > no constructor.
> > 
> > you can't instantiate this class without subclassing it.
> 
> Done.
> 
>
https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
> chrome/browser/google_apis/drive_notification_observer.h:17: virtual
> ~DriveNotificationObserver() {}
> On 2013/04/09 13:30:13, tfarina wrote:
> > can you make this protected?
> 
> Done.
> 
>
https://codereview.chromium.org/13940004/diff/15002/chrome/browser/google_api...
> chrome/browser/google_apis/drive_notification_observer.h:23:
> DISALLOW_COPY_AND_ASSIGN(DriveNotificationObserver);
> On 2013/04/09 13:30:13, tfarina wrote:
> > nor this.
> > 
> > because you have a pure abstract interface. Yeah, I know this disallow the
> copy
> > and assign the subclass, but it's subtle.
> 
> Done.

google_apis LGTM

[commit-bot: I haz the power, 2013-04-10 04:21:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/calvinlo@chromium.org/13940004/17001

[commit-bot: I haz the power, 2013-04-10 05:08:59]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-04-10 08:43:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/calvinlo@chromium.org/13940004/17001

[commit-bot: I haz the power, 2013-04-10 09:17:50]

Change committed as 193356