My favorites | Sign in
Project Home Downloads Wiki Issues Code Search
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 137086: C2P needs a way to know when a user starts and stops sync
5 people starred this issue and may be notified of changes. Back to list
 
Project Member Reported by akalin@chromium.org, Jul 12, 2012
This is so it can add itself as an invalidation listener.

A couple of options:

1. The C2P inherits from ProfileSyncServiceObserver and add itself as an observer to the profile's ProfileSyncService.  Then when it gets a notification, it checks 'PSS::ShouldPushChanges()' which is true iff sync is turned on.

2. The C2P doesn't care if sync is on or not -- PSS exposes methods to add stuff as InvalidationListeners and those listeners will simply not get any invalidations if sync is turned off.

#2 is simpler, but #1 may be needed if C2P needs to know when sync starts for other reasons.

+nyquist for comments.
Jul 12, 2012
#1 akalin@chromium.org
(No comment was entered for this change.)
Blocking: chromium:102709
Jul 12, 2012
#2 akalin@chromium.org
From our meeting, it sounds like C2P does need to know when sync is turned off (and on).

Also, one more note: registering as a PSSObserver doesn't generate a notification, so you'd want to query ShouldPushChanges() immediately after registering.
Jul 20, 2012
#3 msw@chromium.org
ChromeToMobileService should know when sync start/stops, and should be disabled along with Sync.
Otherwise its list of mobile devices may become stale and sending pages will fail.
I didn't see anything promising at content/public/browser/notification_types.h, 

The alternative would be to fallback to a polling mechanism (at regular intervals or on failures).
I think disabling is better, especially if we plan to retain the mobile device list across browser sessions.
Cc: msw@chromium.org
Jul 20, 2012
#4 akalin@chromium.org
@msw: did you miss the description of how to listen for sync in the first comment?
Jul 20, 2012
#5 msw@chromium.org
Ah, I didn't realize that's already in place and waiting on Tommy (or myself) to take action (I wasn't even CC'ed).
Tommy, are you handling this or need help?
Cc: astra...@chromium.org
Jul 20, 2012
#6 akalin@chromium.org
@msw: sorry, I guess I was confused about who was handling what.

I think Tommy is on vacation?
Jul 20, 2012
#7 msw@chromium.org
I'll take a look, thanks.
Owner: msw@chromium.org
Cc: -msw@chromium.org nyquist@chromium.org
Jul 24, 2012
#8 srika...@chromium.org
(No comment was entered for this change.)
Labels: Mstone-23
Aug 20, 2012
#9 msw@chromium.org
AFAIK, syncer::SyncNotifierObserver::OnNotifications[Enabled|Disabled] should be sufficient.
Unfortunately, that's not entirely true from some quick tests:

1) Calls on sign in: disabled, enabled
(final state okay, unnecessary disabled call?)
2) Calls on unplugging the network cable: disabled, disabled, disabled, disabled, disabled
(final state okay, redundant disabled calls?)
3) Calls on plugging in the network cable: disabled, disabled, enabled
(final state okay, unnecessary/redundant disabled calls?)
4) Calls on chrome://settings -> "Disconnect your Google account": <NONE!>
(final state incorrect, no disabled call received)

Reassigning this bug to Fred for verification/analysis.
Owner: akalin@chromium.org
Cc: msw@chromium.org tim@chromium.org dcheng@chromium.org
Aug 21, 2012
#10 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=152609

------------------------------------------------------------------------
r152609 | msw@chromium.org | 2012-08-21T19:37:02.955833Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_unittest.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.h?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/DEPS?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.h?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/location_bar/location_bar_view.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/location_bar_view_gtk.cc?r1=152609&r2=152608&pathrev=152609
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.h?r1=152609&r2=152608&pathrev=152609

Integrate invalidation API into ChromeToMobileService.

Update chrome/browser/DEPS with new dependencies:
(google/cacheinvalidation and sync/notifier)

Observe Sync Notifier invalidation notifications:
(depend on this service for mobile list updates)
(refresh the device list on cloud print invalidation)

Remove RequestMobileListUpdate, timestamp, & account info:
(no longer needed with invalidation integration)
(just set command state and icon visibility w/HasMobiles)

Lazily init the access token, queue search/send operations:
(only get an access token as needed, add |task_queue_|)

Allow concurrent cloud print device search requests:
(handle user-triggered updates while fetching the list)

Misc cleanup (CloudPrintUrl handling, tests, etc.)

TODO(followup): Additional logging, tests, invalidation ack.

BUG=102709,137086

Review URL: https://chromiumcodereview.appspot.com/10834203
------------------------------------------------------------------------
Aug 21, 2012
#11 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=152613

------------------------------------------------------------------------
r152613 | msw@chromium.org | 2012-08-21T20:01:31.034666Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_unittest.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.h?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/DEPS?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.h?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/location_bar/location_bar_view.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/location_bar_view_gtk.cc?r1=152613&r2=152612&pathrev=152613
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.h?r1=152613&r2=152612&pathrev=152613

Revert 152609 - Integrate invalidation API into ChromeToMobileService.
Failed? Win7 Tests (dbg)(6) browser_tests InProcessBrowserTest.Empty:
http://build.chromium.org/p/chromium/builders/Win7%20Tests%20%28dbg%29%286%29/builds/10278/steps/browser_tests/logs/stdio 

Update chrome/browser/DEPS with new dependencies:
(google/cacheinvalidation and sync/notifier)

Observe Sync Notifier invalidation notifications:
(depend on this service for mobile list updates)
(refresh the device list on cloud print invalidation)

Remove RequestMobileListUpdate, timestamp, & account info:
(no longer needed with invalidation integration)
(just set command state and icon visibility w/HasMobiles)

Lazily init the access token, queue search/send operations:
(only get an access token as needed, add |task_queue_|)

Allow concurrent cloud print device search requests:
(handle user-triggered updates while fetching the list)

Misc cleanup (CloudPrintUrl handling, tests, etc.)

TODO(followup): Additional logging, tests, invalidation ack.

BUG=102709,137086

Review URL: https://chromiumcodereview.appspot.com/10834203

TBR=msw@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10861038
------------------------------------------------------------------------
Aug 21, 2012
#12 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=152735

------------------------------------------------------------------------
r152735 | msw@chromium.org | 2012-08-22T04:16:09.588314Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_unittest.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.h?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/DEPS?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.h?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/location_bar/location_bar_view.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/location_bar_view_gtk.cc?r1=152735&r2=152734&pathrev=152735
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.h?r1=152735&r2=152734&pathrev=152735

Reland Integrate invalidation API into ChromeToMobileService.
A continuation of http://codereview.chromium.org/10834203

r152609 reverted original r152609 for debug test crashes:
http://build.chromium.org/p/chromium/builders/Mac%2010.7%20Tests%20%28dbg%29%284%29/builds/100/steps/browser_tests/logs/stdio

Move dtor logic to a ProfileKeyedService::Shutdown OVERRIDE.
This fixes the problem locally (we have no debug trybots?).

Remaining description matches that of the original CL:
======================================================

Update chrome/browser/DEPS with new dependencies:
(google/cacheinvalidation and sync/notifier)

Observe Sync Notifier invalidation notifications:
(depend on this service for mobile list updates)
(refresh the device list on cloud print invalidation)

Remove RequestMobileListUpdate, timestamp, & account info:
(no longer needed with invalidation integration)
(just set command state and icon visibility w/HasMobiles)

Lazily init the access token, queue search/send operations:
(only get an access token as needed, add |task_queue_|)

Allow concurrent cloud print device search requests:
(handle user-triggered updates while fetching the list)

Misc cleanup (CloudPrintUrl handling, tests, etc.)

TODO(followup): Additional logging, tests, invalidation ack.

BUG=102709, 137086
TEST=Chrome To Mobile devices update with cloud print!

Review URL: https://chromiumcodereview.appspot.com/10869002
------------------------------------------------------------------------
Aug 21, 2012
#13 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=152745

------------------------------------------------------------------------
r152745 | msw@chromium.org | 2012-08-22T06:24:27.510229Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_unittest.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.h?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/DEPS?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.h?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/location_bar/location_bar_view.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/location_bar_view_gtk.cc?r1=152745&r2=152744&pathrev=152745
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.h?r1=152745&r2=152744&pathrev=152745

Revert 152735 - Reland Integrate invalidation API into ChromeToMobileService.
A continuation of http://codereview.chromium.org/10834203

Causing a Mac clobber bot build break:
/b/build/slave/Mac/build/src/chrome/browser/chrome_to_mobile_service.cc:42:10: fatalerror: 'google/cacheinvalidation/types.pb.h' file not found
http://build.chromium.org/p/chromium/builders/Mac/builds/16802/steps/compile/logs/stdio#error1

r152609 reverted original r152609 for debug test crashes:
http://build.chromium.org/p/chromium/builders/Mac%2010.7%20Tests%20%28dbg%29%284%29/builds/100/steps/browser_tests/logs/stdio

Move dtor logic to a ProfileKeyedService::Shutdown OVERRIDE.
This fixes the problem locally (we have no debug trybots?).

Remaining description matches that of the original CL:
======================================================

Update chrome/browser/DEPS with new dependencies:
(google/cacheinvalidation and sync/notifier)

Observe Sync Notifier invalidation notifications:
(depend on this service for mobile list updates)
(refresh the device list on cloud print invalidation)

Remove RequestMobileListUpdate, timestamp, & account info:
(no longer needed with invalidation integration)
(just set command state and icon visibility w/HasMobiles)

Lazily init the access token, queue search/send operations:
(only get an access token as needed, add |task_queue_|)

Allow concurrent cloud print device search requests:
(handle user-triggered updates while fetching the list)

Misc cleanup (CloudPrintUrl handling, tests, etc.)

TODO(followup): Additional logging, tests, invalidation ack.

BUG=102709, 137086
TEST=Chrome To Mobile devices update with cloud print!

Review URL: https://chromiumcodereview.appspot.com/10869002

TBR=msw@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10870007
------------------------------------------------------------------------
Aug 22, 2012
#14 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=152801

------------------------------------------------------------------------
r152801 | msw@chromium.org | 2012-08-22T19:34:48.190423Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_unittest.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/chrome_to_mobile_bubble_view.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/pref_names.h?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/location_bar/location_bar_view_mac.mm?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.h?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/chrome_to_mobile_bubble_gtk.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/views/location_bar/location_bar_view.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/gtk/location_bar_view_gtk.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/DEPS?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller_unittest.mm?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/cocoa/chrome_to_mobile_bubble_controller.mm?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.cc?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service_factory.h?r1=152801&r2=152800&pathrev=152801
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/chrome_browser.gypi?r1=152801&r2=152800&pathrev=152801

Re-reland Integrate invalidation API into ChromeToMobileService.
A continuation of http://codereview.chromium.org/10869002

r152745 reverted 2nd attempt r152735 for Mac clobber build break:
/b/build/slave/Mac/build/src/chrome/browser/chrome_to_mobile_service.cc:42:10: fatalerror: 'google/cacheinvalidation/types.pb.h' file not found
http://build.chromium.org/p/chromium/builders/Mac/builds/16802/steps/compile/logs/stdio#error1

Add cacheinvalidation[_proto_cpp] dependency to chrome_browser.gypi.
This fixes my xcodebuild locally (not repro through ninja, etc.)

Remaining description matches that of the second CL:
====================================================
Reland Integrate invalidation API into ChromeToMobileService.
A continuation of http://codereview.chromium.org/10834203

r152609 reverted original r152609 for debug test crashes:
http://build.chromium.org/p/chromium/builders/Mac%2010.7%20Tests%20%28dbg%29%284%29/builds/100/steps/browser_tests/logs/stdio

Move dtor logic to a ProfileKeyedService::Shutdown OVERRIDE.
This fixes the problem locally (we have no debug trybots?).

Remaining description matches that of the original CL:
======================================================
Integrate invalidation API into ChromeToMobileService.

Update chrome/browser/DEPS with new dependencies:
(google/cacheinvalidation and sync/notifier)

Observe Sync Notifier invalidation notifications:
(depend on this service for mobile list updates)
(refresh the device list on cloud print invalidation)

Remove RequestMobileListUpdate, timestamp, & account info:
(no longer needed with invalidation integration)
(just set command state and icon visibility w/HasMobiles)

Lazily init the access token, queue search/send operations:
(only get an access token as needed, add |task_queue_|)

Allow concurrent cloud print device search requests:
(handle user-triggered updates while fetching the list)

Misc cleanup (CloudPrintUrl handling, tests, etc.)

TODO(followup): Additional logging, tests, invalidation ack.

BUG=102709,137086,133352
TEST=Chrome To Mobile devices update with cloud print!
TBR=sky@chromium.org,erg@chromium.org,sail@chromium.org,akalin@chromium.org,thakis@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10876009
------------------------------------------------------------------------
Aug 24, 2012
#15 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=153309

------------------------------------------------------------------------
r153309 | msw@chromium.org | 2012-08-24T22:07:25.300873Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/chrome_to_mobile_service.cc?r1=153309&r2=153308&pathrev=153309

Remove invalid Chrome To Mobile DCHECK, infer notifications are enabled on receipt.

DCHECK from http://crrev.com/152801 may fail in known acceptable cases:
See ctor: // TODO(msw|akalin): Initialize |sync_invalidation_enabled_| properly.
OnIncomingNotification may be called before/after OnNotifications[En|Dis]abled.
This is related to (and partially blocked by) Issues 142475 and 137086.

BUG=144589,142475,137086
TEST=No crash, no send failures due to notifications-enabled false positive.
TBR=sky@chromium.org

Review URL: https://chromiumcodereview.appspot.com/10874058
------------------------------------------------------------------------
Sep 10, 2012
#16 akalin@chromium.org
Repurposing this bug to address #9.
Labels: Feature-Invalidations
Oct 2, 2012
#17 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=159647

------------------------------------------------------------------------
r159647 | akalin@chromium.org | 2012-10-02T06:57:57.499183Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_preference_unittest.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_autofill_unittest.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_unittest.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_password_unittest.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_startup_unittest.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_session_unittest.cc?r1=159647&r2=159646&pathrev=159647
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_typed_url_unittest.cc?r1=159647&r2=159646&pathrev=159647

[Sync] Require ProfileSyncService to have an uninitialized backend on shutdown

BUG=137086


Review URL: https://chromiumcodereview.appspot.com/11037007
------------------------------------------------------------------------
Oct 5, 2012
#18 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=160420

------------------------------------------------------------------------
r160420 | akalin@chromium.org | 2012-10-05T18:03:14.543184Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/sync/notifier/invalidator_test_template.h?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_unittest.cc?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/signin/signin_tracker_unittest.cc?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service.cc?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/sync/notifier/invalidator_registrar_unittest.cc?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/sync/notifier/invalidator.h?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/sync/notifier/invalidator_registrar.cc?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service.h?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/sync/notifier/invalidator_registrar.h?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/invalidation_frontend.h?r1=160420&r2=160419&pathrev=160420
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/ui/webui/sync_setup_handler_unittest.cc?r1=160420&r2=160419&pathrev=160420

[Invalidations] Require there to be no registered handlers on Invalidator destruction

Add CHECK to catch sloppy clients.

Make ProfileSyncService destroy its invalidator registrar on shut down (also to catch
sloppy clients).  Comment on expected usage of Initialize() and Shutdown(),
and add DCHECKs for them.

Fix Invalidator test template to unregister handlers properly.  Also fix some ProfileSyncService tests.

BUG=137086
TBR=tim@chromium.org,arv@chromium.org

Review URL: https://codereview.chromium.org/11046008
------------------------------------------------------------------------
Oct 5, 2012
#19 bugdro...@chromium.org
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=160525

------------------------------------------------------------------------
r160525 | akalin@chromium.org | 2012-10-06T00:04:49.624261Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service_unittest.cc?r1=160525&r2=160524&pathrev=160525
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service.cc?r1=160525&r2=160524&pathrev=160525
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/sync/profile_sync_service.h?r1=160525&r2=160524&pathrev=160525

[Sync] Notify invalidation handlers when sync is disabled/enabled

BUG=137086


Review URL: https://chromiumcodereview.appspot.com/11043012
------------------------------------------------------------------------
Oct 8, 2012
#20 akalin@chromium.org
This should be fixed.  msw@, can you verify that your steps in #9 now work?
Owner: msw@chromium.org
Oct 10, 2012
#21 ka...@chromium.org
Moving all non essential bugs to the next Milestone
Labels: -Mstone-23 MovedFrom-23 Mstone-24
Oct 10, 2012
#22 msw@chromium.org
Taking another look, on ToT 24.0.1292.0 (Developer Build 161217):
1) Calls on sign in: 2x TRANSIENT_INVALIDATION_ERROR, INVALIDATIONS_ENABLED
(final state okay, unnecessary disabled calls)
2) Calls on unplugging the network cable: 6x+ TRANSIENT_INVALIDATION_ERROR (called every few seconds)
(final state okay, redundant disabled calls)
3) Calls on plugging in the network cable: 2x TRANSIENT_INVALIDATION_ERROR, 1x INVALIDATIONS_ENABLED
(final state okay, unnecessary disabled calls)
4) Calls on chrome://settings -> "Disconnect your Google account": 2x TRANSIENT_INVALIDATION_ERROR
(final state okay, redundant disabled calls)

The biggest issue here (the final state for signing out) is fixed, afaict; thanks!
It'd be nice (but not critical) if OnInvalidatorStateChange was only called on actual state changes.
Oct 12, 2012
#23 akalin@chromium.org
I'd prefer that invalidation clients debounce the signal themselves if needed.  But I think most of the redundancy comes from the underlying XMPP connection trying to reconnect with exp. backoff.

Release folks: This has baked in canary for a while.  Can we merge it?

Note that there are three chained CLs: 159647 160420 160525
Owner: akalin@chromium.org
Labels: -MovedFrom-23 -Mstone-24 Mstone-23 ReleaseBlock-Stable
Oct 12, 2012
#24 msw@chromium.org
Client debouncing sgtm, but it should be documented at syncer::InvalidationHandler.
Oct 15, 2012
#25 kar...@google.com
talked to fred. this is m24 :)
Labels: -Mstone-23 -ReleaseBlock-Stable Mstone-24
Oct 15, 2012
#26 akalin@chromium.org
Not important enough to block m23
Oct 23, 2012
#27 tim@chromium.org
(No comment was entered for this change.)
Labels: -Feature-Invalidations Feature-Sync-Invalidation
Dec 29, 2012
#28 akalin@chromium.org
Looks fixed.
Status: Fixed
Mar 10, 2013
#29 bugdro...@chromium.org
(No comment was entered for this change.)
Labels: -Area-internals -Feature-Sync -Feature-ChromeToMobile -Mstone-24 -Feature-Sync-Invalidation Cr-Services-Sync Cr-Internals M-24 Cr-Services-ChromeToMobile Cr-Services-Invalidation
Sign in to add a comment

Powered by Google Project Hosting