Add shutdown notification and service refs to SyncServiceObserver.

Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

TBR=sdefresne,treib

BUG=

Review-Url: https://codereview.chromium.org/2657673004
Cr-Commit-Position: refs/heads/master@{#447902}
Committed: https://chromium.googlesource.com/chromium/src/+/4ef35706c091a8f8f18fbee2c5672831ad24ecea

[Steven Holte, 2017-01-26 00:48:33]

holte@chromium.org changed reviewers:
+ skym@chromium.org

[skym, 2017-01-26 17:00:37]

Thanks for making these changes! My main concern is the
ShutdownImpl()/NotifyShutdown() ordering. I'm okay if you want to initially only
listen to OnSyncConfigurationCompleted(), up to you.

https://codereview.chromium.org/2657673004/diff/1/components/browser_sync/pro...
File components/browser_sync/profile_sync_service.cc (right):

https://codereview.chromium.org/2657673004/diff/1/components/browser_sync/pro...
components/browser_sync/profile_sync_service.cc:716: NotifyShutdown();
Having this right before ShutdownImpl(...), which does call NotifyObservers()
worries me a little bit. While I'm suspicious this doesn't break any of the
existing observer's functionality, it does kind of go against the documentation
in sync_service_observer.h.

I think I'd prefer it if you swapped these two, unless there's a reason for this
ordering.

https://codereview.chromium.org/2657673004/diff/1/components/browser_sync/pro...
components/browser_sync/profile_sync_service.cc:721:
RemoveObserver(sync_error_controller_.get());
Ugh. I guess this could be done with the OnSyncShutdown() event now. Not sure
why it wasn't originally done in the SyncErrorController's dtor. Not sure which
is better. I don't really understand why we have to manually reset it if we're
going to shutdown anyways. Regardless, out of scope of your CL.

https://codereview.chromium.org/2657673004/diff/1/components/sync/driver/sync...
File components/sync/driver/sync_service_observer.h (right):

https://codereview.chromium.org/2657673004/diff/1/components/sync/driver/sync...
components/sync/driver/sync_service_observer.h:33: virtual void
OnSyncConfigurationCompleted(SyncService* sync) {}
So I'm not sure what other checks/events you're listening to, but I don't think
is going to suffice for you. I ran into a similar problem recently when I wanted
to remove sessions suggestions when the user signs out/disables sync.
https://codereview.chromium.org/2578293002/diff/160001/components/ntp_snippet...

Configuration happens when you change the set of types enabled types. But you
need to still have at least one enabled type for configuration to run. If you
end up disabling all the types or signing out, this event doesn't get fired.

The solution is to listen to OnStateChange(). I think we want to update all
these functions to pass a SyncService* for consistency anyways. Though I'd be
willing to do this if you want, you're already improving things significantly
imo.

[Steven Holte, 2017-01-26 20:31:47]

https://codereview.chromium.org/2657673004/diff/1/components/browser_sync/pro...
File components/browser_sync/profile_sync_service.cc (right):

https://codereview.chromium.org/2657673004/diff/1/components/browser_sync/pro...
components/browser_sync/profile_sync_service.cc:716: NotifyShutdown();
On 2017/01/26 17:00:37, skym wrote:
> Having this right before ShutdownImpl(...), which does call NotifyObservers()
> worries me a little bit. While I'm suspicious this doesn't break any of the
> existing observer's functionality, it does kind of go against the
documentation
> in sync_service_observer.h.
> 
> I think I'd prefer it if you swapped these two, unless there's a reason for
this
> ordering.

Done.

https://codereview.chromium.org/2657673004/diff/1/components/browser_sync/pro...
components/browser_sync/profile_sync_service.cc:721:
RemoveObserver(sync_error_controller_.get());
On 2017/01/26 17:00:37, skym wrote:
> Ugh. I guess this could be done with the OnSyncShutdown() event now. Not sure
> why it wasn't originally done in the SyncErrorController's dtor. Not sure
which
> is better. I don't really understand why we have to manually reset it if we're
> going to shutdown anyways. Regardless, out of scope of your CL.

Acknowledged.

https://codereview.chromium.org/2657673004/diff/1/components/sync/driver/sync...
File components/sync/driver/sync_service_observer.h (right):

https://codereview.chromium.org/2657673004/diff/1/components/sync/driver/sync...
components/sync/driver/sync_service_observer.h:33: virtual void
OnSyncConfigurationCompleted(SyncService* sync) {}
On 2017/01/26 17:00:37, skym wrote:
> So I'm not sure what other checks/events you're listening to, but I don't
think
> is going to suffice for you. I ran into a similar problem recently when I
wanted
> to remove sessions suggestions when the user signs out/disables sync.
>
https://codereview.chromium.org/2578293002/diff/160001/components/ntp_snippet...
> 
> Configuration happens when you change the set of types enabled types. But you
> need to still have at least one enabled type for configuration to run. If you
> end up disabling all the types or signing out, this event doesn't get fired.
> 
> The solution is to listen to OnStateChange(). I think we want to update all
> these functions to pass a SyncService* for consistency anyways. Though I'd be
> willing to do this if you want, you're already improving things significantly
> imo.

Added SyncService* to all functions.
Also made a default impl here for OnStateChanged, so it can be omitted in some
subclasses.

[skym, 2017-01-26 23:43:23]

Thanks for doing this! lgtm

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/auth_sync_observer.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/auth_sync_observer.cc:53:
browser_sync::ProfileSyncService* sync_service =
This variable can be replaced by |sync|.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/external_pref_loader.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/external_pref_loader.cc:158:
browser_sync::ProfileSyncService* service =
This variable can be replaced by |sync|.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/external_pref_loader.cc:160: DCHECK(service);
I'd remove the DCHECK as well while you're in here.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/supervis...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/supervis...
chrome/browser/supervised_user/supervised_user_service.cc:1261:
browser_sync::ProfileSyncService* service =
This variable can be replaced by |sync|.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/sync_fil...
File chrome/browser/sync_file_system/sync_file_system_service.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/sync_fil...
chrome/browser/sync_file_system/sync_file_system_service.cc:732:
syncer::SyncService* profile_sync_service =
This variable can be replaced by |sync|.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/sync_fil...
chrome/browser/sync_file_system/sync_file_system_service.cc:734: if
(profile_sync_service)
Same for this if

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:957:
OnStateChanged(NULL);
Should use nullptr, not NULL. I'm not familiar at all with this code, but it
might make more sense to call UpdateSyncState() everywhere. OnStateChange() is
really generic sounding, doesn't give as much context to the call sites.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:962:
OnStateChanged(NULL);
Same as above.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:1125:
OnStateChanged(NULL);
Same as above.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:1377:
OnStateChanged(NULL);
Same as above.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:86:
sync_service_->RemoveObserver(this);
So many of these uses could benefit from the ScopedObserver you used in your
other CL. Of course, far outside the scope of this CL.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
(right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc:258:
OnStateChanged(NULL);
nullptr

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc:303:
OnStateChanged(NULL);
nullptr

https://codereview.chromium.org/2657673004/diff/20001/components/suggestions/...
File components/suggestions/suggestions_service_impl.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/components/suggestions/...
components/suggestions/suggestions_service_impl.cc:212: OnStateChanged(NULL);
nullptr

[Steven Holte, 2017-01-27 02:27:19]

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/chromeos...
File chrome/browser/chromeos/login/signin/auth_sync_observer.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/chromeos...
chrome/browser/chromeos/login/signin/auth_sync_observer.cc:53:
browser_sync::ProfileSyncService* sync_service =
On 2017/01/26 23:43:22, skym wrote:
> This variable can be replaced by |sync|.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/external_pref_loader.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/external_pref_loader.cc:158:
browser_sync::ProfileSyncService* service =
On 2017/01/26 23:43:22, skym wrote:
> This variable can be replaced by |sync|.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/external_pref_loader.cc:160: DCHECK(service);
On 2017/01/26 23:43:23, skym wrote:
> I'd remove the DCHECK as well while you're in here.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/supervis...
File chrome/browser/supervised_user/supervised_user_service.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/supervis...
chrome/browser/supervised_user/supervised_user_service.cc:1261:
browser_sync::ProfileSyncService* service =
On 2017/01/26 23:43:23, skym wrote:
> This variable can be replaced by |sync|.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/sync_fil...
File chrome/browser/sync_file_system/sync_file_system_service.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/sync_fil...
chrome/browser/sync_file_system/sync_file_system_service.cc:732:
syncer::SyncService* profile_sync_service =
On 2017/01/26 23:43:23, skym wrote:
> This variable can be replaced by |sync|.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/sync_fil...
chrome/browser/sync_file_system/sync_file_system_service.cc:734: if
(profile_sync_service)
On 2017/01/26 23:43:23, skym wrote:
> Same for this if

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/browser_options_handler.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:957:
OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> Should use nullptr, not NULL. I'm not familiar at all with this code, but it
> might make more sense to call UpdateSyncState() everywhere. OnStateChange() is
> really generic sounding, doesn't give as much context to the call sites.

Changed to UpdateSyncState().

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:962:
OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> Same as above.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:1125:
OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> Same as above.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/browser_options_handler.cc:1377:
OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> Same as above.

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
(right):

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc:258:
OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> nullptr

Done.

https://codereview.chromium.org/2657673004/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc:303:
OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> nullptr

Done.

https://codereview.chromium.org/2657673004/diff/20001/components/suggestions/...
File components/suggestions/suggestions_service_impl.cc (right):

https://codereview.chromium.org/2657673004/diff/20001/components/suggestions/...
components/suggestions/suggestions_service_impl.cc:212: OnStateChanged(NULL);
On 2017/01/26 23:43:23, skym wrote:
> nullptr

Changed to sync_service_

[Steven Holte, 2017-01-27 02:39:13]

Description was changed from

==========
Add shutdown notification and service refs to SyncServiceObserver.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

BUG=
==========

to

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

BUG=
==========

[Steven Holte, 2017-01-27 02:39:44]

holte@chromium.org changed reviewers:
+ sdefresne@chromium.org, sky@chromium.org

[Steven Holte, 2017-01-27 02:39:44]

+sky for chrome/browser
+sdefresne for components/ (other than c/sync/ c/browser_sync/)

[sky, 2017-01-27 16:29:44]

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/external_pref_loader.cc (right):

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/external_pref_loader.cc:158: if
(!sync->CanSyncStart()) {
remove {}

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:426:
OnStateChanged(nullptr);
This seems error prone. I recommend having OnStateChanged() and this call a
function with no args. Maybe UpdateSyncState() as other files have.

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
(right):

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc:258:
OnStateChanged(nullptr);
Same comment above using another function.

[Steven Holte, 2017-01-27 20:11:45]

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/extensio...
File chrome/browser/extensions/external_pref_loader.cc (right):

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/extensio...
chrome/browser/extensions/external_pref_loader.cc:158: if
(!sync->CanSyncStart()) {
On 2017/01/27 16:29:44, sky wrote:
> remove {}

Done.

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/options/clear_browser_data_handler.cc (right):

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/options/clear_browser_data_handler.cc:426:
OnStateChanged(nullptr);
On 2017/01/27 16:29:44, sky wrote:
> This seems error prone. I recommend having OnStateChanged() and this call a
> function with no args. Maybe UpdateSyncState() as other files have.

Done.

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc
(right):

https://codereview.chromium.org/2657673004/diff/40001/chrome/browser/ui/webui...
chrome/browser/ui/webui/settings/settings_clear_browsing_data_handler.cc:258:
OnStateChanged(nullptr);
On 2017/01/27 16:29:44, sky wrote:
> Same comment above using another function.

Done.

[sky, 2017-01-27 23:38:03]

LGTM

[Steven Holte, 2017-01-28 00:07:34]

holte@chromium.org changed reviewers:
+ stevenjb@chromium.org

[Steven Holte, 2017-01-28 00:07:34]

+stevenjb for chrome/browser/ui/webui/options

[stevenjb, 2017-01-28 01:01:00]

ui/webui/options|settings lgtm

[Steven Holte, 2017-01-30 19:14:05]

holte@chromium.org changed reviewers:
+ mkwst@chromium.org, treib@chromium.org

[Steven Holte, 2017-01-30 19:14:06]

+treib for components/suggestions and components/ntp_snippets

+mkwst for components/browsing_data and components/password_manager

[Steven Holte, 2017-02-01 20:06:04]

Description was changed from

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

BUG=
==========

to

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

TBR=mkwst,treib

BUG=
==========

[Steven Holte, 2017-02-01 20:08:05]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-02-01 20:08:06]

The patchset sent to the CQ was uploaded after l-g-t-m from skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2657673004/#ps60001
(title: "UpdateSyncState")

[commit-bot: I haz the power, 2017-02-01 20:08:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 20:11:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 20:11:52]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Steven Holte, 2017-02-01 21:10:21]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-02-01 21:10:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, sky@chromium.org, skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2657673004/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2017-02-01 21:10:50]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 21:19:50]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 21:19:52]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Steven Holte, 2017-02-01 22:00:47]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-02-01 22:00:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, sky@chromium.org, skym@chromium.org
Link to the patchset: https://codereview.chromium.org/2657673004/#ps100001
(title: "Minor test fixes")

[commit-bot: I haz the power, 2017-02-01 22:01:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 22:09:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 22:09:32]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[Steven Holte, 2017-02-01 22:31:31]

The CQ bit was checked by holte@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-01 22:32:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 22:41:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 22:41:38]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Steven Holte, 2017-02-01 23:15:18]

The CQ bit was checked by holte@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-01 23:15:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 23:41:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-01 23:41:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Steven Holte, 2017-02-02 20:23:16]

The CQ bit was checked by holte@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 20:23:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 20:38:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 20:38:53]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Mike West, 2017-02-02 20:50:24]

On 2017/01/30 at 19:14:06, holte wrote:
> +mkwst for components/browsing_data and components/password_manager

LGTM.

[Steven Holte, 2017-02-02 21:37:26]

The CQ bit was checked by holte@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 21:37:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 21:55:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 21:55:42]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Steven Holte, 2017-02-02 22:26:55]

The CQ bit was checked by holte@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 22:27:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-02 23:08:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 23:08:21]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Steven Holte, 2017-02-02 23:29:54]

The CQ bit was checked by holte@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 23:30:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 00:52:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 00:52:14]

Dry run: This issue passed the CQ dry run.

[Steven Holte, 2017-02-03 02:47:44]

Description was changed from

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

TBR=mkwst,treib

BUG=
==========

to

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

TBR=sdefresne,treib

BUG=
==========

[Steven Holte, 2017-02-03 02:50:08]

TBR +sdefresne for ios/chrome/

[Steven Holte, 2017-02-03 02:50:22]

The CQ bit was checked by holte@chromium.org

[Steven Holte, 2017-02-03 02:50:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
stevenjb@chromium.org, sky@chromium.org, skym@chromium.org, mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/2657673004/#ps220001
(title: "Chromeos fix")

[commit-bot: I haz the power, 2017-02-03 02:50:49]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 03:30:14]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1486090222059260,
"parent_rev": "652129ce6e65ee501a16614a9ec44b830a56ca0c", "commit_rev":
"4ef35706c091a8f8f18fbee2c5672831ad24ecea"}

[commit-bot: I haz the power, 2017-02-03 03:30:47]

Description was changed from

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

TBR=sdefresne,treib

BUG=
==========

to

==========
Add shutdown notification and service refs to SyncServiceObserver.

Modifies ServiceSyncObserver to recieve SyncService* to the notifying
service in all of it's methods.

Also adds OnShutdown() notification.

This will support adding a long-lived observer in
https://codereview.chromium.org/2653693004/

TBR=sdefresne,treib

BUG=

Review-Url: https://codereview.chromium.org/2657673004
Cr-Commit-Position: refs/heads/master@{#447902}
Committed:
https://chromium.googlesource.com/chromium/src/+/4ef35706c091a8f8f18fbee2c567...
==========

[commit-bot: I haz the power, 2017-02-03 03:30:49]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/4ef35706c091a8f8f18fbee2c567...