Integrate invalidation API into ChromeToMobileService.

chrome/browser/chrome_to_mobile_service.cc

Index: chrome/browser/chrome_to_mobile_service.cc
diff --git a/chrome/browser/chrome_to_mobile_service.cc b/chrome/browser/chrome_to_mobile_service.cc
index bc80c393f6f95ba6a41815d2b1f899944d7746f0..d001999ee7b3221424e8cc6b1c20a9b4c531fa21 100644
--- a/chrome/browser/chrome_to_mobile_service.cc
+++ b/chrome/browser/chrome_to_mobile_service.cc
@@ -11,15 +11,15 @@
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/metrics/histogram.h"
-#include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/app/chrome_command_ids.h"
-#include "chrome/browser/content_settings/cookie_settings.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/printing/cloud_print/cloud_print_url.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/token_service.h"
 #include "chrome/browser/signin/token_service_factory.h"
+#include "chrome/browser/sync/profile_sync_service.h"
+#include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_command_controller.h"
 #include "chrome/browser/ui/browser_finder.h"
@@ -38,10 +38,13 @@
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_source.h"
 #include "content/public/browser/web_contents.h"
+#include "google/cacheinvalidation/include/types.h"
+#include "google/cacheinvalidation/types.pb.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_request_context_getter.h"
+#include "sync/notifier/invalidation_util.h"
 
 namespace {
 
@@ -59,19 +62,16 @@ const size_t kAuthRetryDelayHours = 6;
 // Note that this limitation does not hold across application restarts.
 const int kSearchRequestDelayHours = 24;
 
+// The sync invalidation object ID for Chrome to Mobile's mobile device list.
+// This corresponds with cloud print's server-side invalidation object ID.
+// Meaning: "U" == "User", "CM" == "Chrome to Mobile", "MLST" == "Mobile LiST".
+const char kSyncInvalidationObjectIdChromeToMobileDeviceList[] = "UCMMLST";
+
 // The cloud print OAuth2 scope and 'printer' type of compatible mobile devices.
 const char kCloudPrintAuth[] = "https://www.googleapis.com/auth/cloudprint";
 const char kTypeAndroid[] = "ANDROID_CHROME_SNAPSHOT";
 const char kTypeIOS[] = "IOS_CHROME_SNAPSHOT";
 
-// The account info URL pattern and strings to check for cloud print access.
-// The 'key=' query parameter is used for caching; supply a random number.
-// The 'rv=2' query parameter requests a JSON response; use 'rv=1' for XML.
-const char kAccountInfoURL[] =
-    "https://clients1.google.com/tbproxy/getaccountinfo?key=%s&rv=2&%s";
-const char kAccountServicesKey[] = "services";
-const char kCloudPrintSerivceValue[] = "cprt";
-
 // The Chrome To Mobile requestor type; used by services for filtering.
 const char kChromeToMobileRequestor[] = "requestor=chrome-to-mobile";
 
@@ -180,44 +180,53 @@ bool ChromeToMobileService::IsChromeToMobileEnabled() {
 void ChromeToMobileService::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterListPref(prefs::kChromeToMobileDeviceList,
                           PrefService::UNSYNCABLE_PREF);
-  prefs->RegisterInt64Pref(prefs::kChromeToMobileTimestamp, 0,

[nyquist, 2012/08/21 00:00:01]

Remove from chrome/common/pref_names.[h|cc]

[msw, 2012/08/21 03:37:01]

Done.

-                           PrefService::UNSYNCABLE_PREF);
 }
 
 ChromeToMobileService::ChromeToMobileService(Profile* profile)
     : ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
       profile_(profile),
-      cloud_print_url_(new CloudPrintURL(profile)),
-      cloud_print_accessible_(false) {
-  // TODO(msw): Fix GMock tests, which lack profiles (http://crbug.com/122183).
-  if (profile_) {
-    // Get an access token as soon as the Gaia login refresh token is available.
-    TokenService* service = TokenServiceFactory::GetForProfile(profile_);
-    registrar_.Add(this, chrome::NOTIFICATION_TOKEN_AVAILABLE,
-                   content::Source<TokenService>(service));
-    if (service->HasOAuthLoginToken())
-      RequestAccessToken();
+      profile_sync_service_enabled_(false),
+      sync_invalidation_enabled_(false),
+      cloud_print_url_(new CloudPrintURL(profile)) {
+  // TODO(msw): Unit tests do not provide profiles; see http://crbug.com/122183
+  ProfileSyncService* profile_sync_service =
+      profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
+  if (profile_sync_service) {
+    // Check the profile sync service state, as prescribed by the sync team.
+    profile_sync_service_enabled_ = profile_sync_service->ShouldPushChanges();
+    // Register for profile sync and cloud print device list notifications.
+    profile_sync_service->AddObserver(this);
+    profile_sync_service->RegisterInvalidationHandler(this);
+    // TODO(msw|akalin): Initialize |sync_invalidation_enabled_| properly.
+    syncer::ObjectIdSet ids;
+    ids.insert(invalidation::ObjectId(
+        ipc::invalidation::ObjectSource::CHROME_COMPONENTS,
+        kSyncInvalidationObjectIdChromeToMobileDeviceList));
+    profile_sync_service->UpdateRegisteredInvalidationIds(this, ids);
   }
 }
 
 ChromeToMobileService::~ChromeToMobileService() {
   while (!snapshots_.empty())
     DeleteSnapshot(*snapshots_.begin());
+  // TODO(msw): Unit tests do not provide profiles; see http://crbug.com/122183
+  ProfileSyncService* profile_sync_service =
+      profile_ ? ProfileSyncServiceFactory::GetForProfile(profile_) : NULL;
+  if (profile_sync_service) {
+    // Unregister for profile sync and cloud print device list notifications.
+    profile_sync_service->RemoveObserver(this);
+    profile_sync_service->UnregisterInvalidationHandler(this);
+  }
 }
 
 bool ChromeToMobileService::HasMobiles() const {
-  return !GetMobiles()->empty();
+  const base::ListValue* mobiles = GetMobiles();
+  return mobiles && !mobiles->empty();
 }
 
 const base::ListValue* ChromeToMobileService::GetMobiles() const {
-  return profile_->GetPrefs()->GetList(prefs::kChromeToMobileDeviceList);
-}
-
-void ChromeToMobileService::RequestMobileListUpdate() {
-  if (access_token_.empty())
-    RequestAccessToken();
-  else if (cloud_print_accessible_)
-    RequestDeviceSearch();
+  return (profile_sync_service_enabled_ && sync_invalidation_enabled_) ?
+      profile_->GetPrefs()->GetList(prefs::kChromeToMobileDeviceList) : NULL;
 }
 
 void ChromeToMobileService::GenerateSnapshot(Browser* browser,
@@ -234,19 +243,28 @@ void ChromeToMobileService::GenerateSnapshot(Browser* browser,
   }
 }
 
-void ChromeToMobileService::SendToMobile(const base::DictionaryValue& mobile,
+void ChromeToMobileService::SendToMobile(const base::DictionaryValue* mobile,
                                          const FilePath& snapshot,
                                          Browser* browser,
                                          base::WeakPtr<Observer> observer) {
+  if (access_token_.empty()) {
+    // Enqueue this task to perform after obtaining an access token.
+    task_queue_.push(base::Bind(&ChromeToMobileService::SendToMobile,
+        weak_ptr_factory_.GetWeakPtr(), base::Owned(mobile->DeepCopy()),
+        snapshot, browser, observer));
+    RequestAccessToken();
+    return;
+  }
+
   LogMetric(SENDING_URL);
 
   JobData data;
   std::string mobile_os;
-  if (!mobile.GetString("type", &mobile_os))
+  if (!mobile->GetString("type", &mobile_os))
     NOTREACHED();
   data.mobile_os = (mobile_os.compare(kTypeAndroid) == 0) ?
       ChromeToMobileService::ANDROID : ChromeToMobileService::IOS;
-  if (!mobile.GetString("id", &data.mobile_id))
+  if (!mobile->GetString("id", &data.mobile_id))
     NOTREACHED();
   content::WebContents* web_contents = chrome::GetActiveWebContents(browser);
   data.url = web_contents->GetURL();
@@ -301,11 +319,8 @@ void ChromeToMobileService::LearnMore(Browser* browser) const {
   chrome::Navigate(&params);
 }
 
-void ChromeToMobileService::OnURLFetchComplete(
-    const net::URLFetcher* source) {
-  if (source == account_info_request_.get())
-    HandleAccountInfoResponse();
-  else if (source == search_request_.get())
+void ChromeToMobileService::OnURLFetchComplete(const net::URLFetcher* source) {
+  if (source == search_request_.get())
     HandleSearchResponse();
   else
     HandleSubmitResponse(source);
@@ -318,8 +333,9 @@ void ChromeToMobileService::Observe(
   DCHECK_EQ(type, chrome::NOTIFICATION_TOKEN_AVAILABLE);
   TokenService::TokenAvailableDetails* token_details =
       content::Details<TokenService::TokenAvailableDetails>(details).ptr();
+  // Invalidate the cloud print access token on Gaia login token updates.
   if (token_details->service() == GaiaConstants::kGaiaOAuth2LoginRefreshToken)
-    RequestAccessToken();
+    access_token_.clear();
 }
 
 void ChromeToMobileService::OnGetTokenSuccess(
@@ -329,18 +345,64 @@ void ChromeToMobileService::OnGetTokenSuccess(
   access_token_fetcher_.reset();
   auth_retry_timer_.Stop();
   access_token_ = access_token;
-  RequestAccountInfo();
+
+  while (!task_queue_.empty()) {
+    // Post all tasks that were queued and waiting on a valid access token.
+    if (!content::BrowserThread::PostTask(content::BrowserThread::UI, FROM_HERE,
+                                          task_queue_.front())) {
+      NOTREACHED();
+    }
+    task_queue_.pop();
+  }
 }
 
 void ChromeToMobileService::OnGetTokenFailure(
     const GoogleServiceAuthError& error) {
   access_token_fetcher_.reset();
   auth_retry_timer_.Stop();
+
   auth_retry_timer_.Start(
       FROM_HERE, base::TimeDelta::FromHours(kAuthRetryDelayHours),
       this, &ChromeToMobileService::RequestAccessToken);
 }
 
+void ChromeToMobileService::OnStateChanged() {
+  // Check the profile sync service state, as prescribed by the sync team.
+  profile_sync_service_enabled_ =
+      ProfileSyncServiceFactory::GetForProfile(profile_)->ShouldPushChanges();

[akalin, 2012/08/20 22:29:30]

maybe dcheck return value of GetForProfile()?  I guess if we're in here, the PSS
must exist

[msw, 2012/08/20 22:43:57]

Yup, this ProfileSyncServiceObserver function is only reached iff |profile_| and
its PSS were valid in the ctor. Do we support PSS becoming NULL (but still
sending state changes?) even if the factory
DependsOn(ProfileSyncServiceFactory...)? Otherwise, there's no reason to DCHECK,
since it'll crash anyway in such a fatal scenario.

[tim (not reviewing), 2012/08/21 00:33:47]

Out of curiosity, what exactly do you need to check here?  ShouldPushChanges is
likely sufficient, but if it's necessary I'd like to know why since we have a
few alternatives now.  Do you care if sync is (e.g.) blocked waiting for the
user to type in a password to sync encrypted types, or do you really just care
if the user has signed in to chrome (and hasn't explicitly chosen to sign out
yet)? The SigninManager or IsSyncEnabledAndLogged in could tell you that.

Or maybe you do need ShouldPushChanges?

[nyquist, 2012/08/21 00:38:59]

I believe the intent here is what you propose:
SigninManager::IsSyncEnabledAndLoggedIn().

[akalin, 2012/08/21 01:50:58]

Okay I think I was confused, but now I'm not.  Basically, it sounds like we want
to show the icon exactly when sync notifications are working, for which "sync is
enabled and user is logged in" is a necessary condition.  That is, if sync is
enabled and user is logged in, sync notifications are (most likely) on.  If sync
is disabled or user is not logged in, sync notifications are definitely off.

Anyway, I think it suffices to just keep track of sync_invalidation_enabled_,
i.e. remove the registering as an observer of ProfileSyncService and the
checking of ShouldPushChanges().

My original question was whether C2P could be disabled independently from sync,
but it sounds like that's not the case.  If it were the case, then you'd need
some code like the following:

void OnC2PDisable() {
  ProfileSyncService* pss = ...;
  pss->UpdateRegisteredInvalidationIds(this, ObjectIdSet());
}

[msw, 2012/08/21 03:37:01]

Done. Now it's just tracking OnNotifications[En|Dis]abled.
I updated http://crbug.com/137086 for Fred to check those calls, etc.

+  UpdateCommandState();
+}
+
+void ChromeToMobileService::OnNotificationsEnabled() {
+  sync_invalidation_enabled_ = true;
+  UpdateCommandState();
+}
+
+void ChromeToMobileService::OnNotificationsDisabled(
+    syncer::NotificationsDisabledReason reason) {
+  sync_invalidation_enabled_ = false;
+  UpdateCommandState();
+}
+
+void ChromeToMobileService::OnIncomingNotification(
+    const syncer::ObjectIdPayloadMap& id_payloads,
+    syncer::IncomingNotificationSource source) {
+  DCHECK_EQ(id_payloads.size(), 1U);
+  DCHECK_EQ(id_payloads.count(invalidation::ObjectId(
+      ipc::invalidation::ObjectSource::CHROME_COMPONENTS,
+      kSyncInvalidationObjectIdChromeToMobileDeviceList)), 1U);
+  RequestDeviceSearch();
+}
+
+const std::string& ChromeToMobileService::GetAccessTokenForTest() const {
+  return access_token_;
+}
+
+void ChromeToMobileService::SetAccessTokenForTest(
+    const std::string& access_token) {
+  access_token_ = access_token;
+}
+
 void ChromeToMobileService::UpdateCommandState() const {
   // Ensure the feature is not disabled by commandline options.
   DCHECK(IsChromeToMobileEnabled());
@@ -435,8 +497,13 @@ void ChromeToMobileService::SendRequest(net::URLFetcher* request,
 }
 
 void ChromeToMobileService::RequestAccessToken() {
-  // Deny concurrent requests and bail without a valid Gaia login refresh token.
+  // Register to observe Gaia login refresh token updates.
   TokenService* token_service = TokenServiceFactory::GetForProfile(profile_);
+  if (registrar_.IsEmpty())
+    registrar_.Add(this, chrome::NOTIFICATION_TOKEN_AVAILABLE,
+                   content::Source<TokenService>(token_service));
+
+  // Deny concurrent requests and bail without a valid Gaia login refresh token.
   if (access_token_fetcher_.get() || !token_service->HasOAuthLoginToken())
     return;
 
@@ -450,47 +517,18 @@ void ChromeToMobileService::RequestAccessToken() {
                                std::vector<std::string>(1, kCloudPrintAuth));
 }
 
-void ChromeToMobileService::RequestAccountInfo() {
-  // Deny concurrent requests.
-  if (account_info_request_.get())
-    return;
-
-  std::string url_string = StringPrintf(kAccountInfoURL,
-      base::GenerateGUID().c_str(), kChromeToMobileRequestor);
-  GURL url(url_string);
-
-  // Account information is read from the profile's cookie. If cookies are
-  // blocked, access cloud print directly to list any potential devices.
-  scoped_refptr<CookieSettings> cookie_settings =
-      CookieSettings::Factory::GetForProfile(profile_);
-  if (cookie_settings && !cookie_settings->IsReadingCookieAllowed(url, url)) {
-    cloud_print_accessible_ = true;
-    RequestMobileListUpdate();
-    return;
-  }
-
-  account_info_request_.reset(
-      net::URLFetcher::Create(url, net::URLFetcher::GET, this));
-  account_info_request_->SetRequestContext(profile_->GetRequestContext());
-  account_info_request_->SetMaxRetries(kMaxRetries);
-  // This request sends the user's cookie to check the cloud print service flag.
-  account_info_request_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES);
-  account_info_request_->Start();
-}
-
 void ChromeToMobileService::RequestDeviceSearch() {
-  // Deny requests if cloud print is inaccessible, and deny concurrent requests.
-  if (!cloud_print_accessible_ || search_request_.get())
+  DCHECK(sync_invalidation_enabled_);
+  if (access_token_.empty()) {
+    // Enqueue this task to perform after obtaining an access token.
+    task_queue_.push(base::Bind(&ChromeToMobileService::RequestDeviceSearch,
+                                weak_ptr_factory_.GetWeakPtr()));
+    RequestAccessToken();
     return;
+  }
 
-  PrefService* prefs = profile_->GetPrefs();
-  base::TimeTicks previous_search_time = base::TimeTicks::FromInternalValue(
-      prefs->GetInt64(prefs::kChromeToMobileTimestamp));
-
-  // Deny requests before the delay period has passed since the last request.
-  base::TimeDelta elapsed_time = base::TimeTicks::Now() - previous_search_time;
-  if (!previous_search_time.is_null() &&
-      elapsed_time.InHours() < kSearchRequestDelayHours)
+  // Deny concurrent requests.
+  if (search_request_.get())

[nyquist, 2012/08/20 23:19:19]

If a user does cleanup in her printer list by deleting/renaming, etc., we might
miss the last update if we already have one in process (which might or might not
get the latest data).
Could we instead queue another search?

[msw, 2012/08/21 03:37:01]

Done; good point. Some users will definitely manage to update their devices
while we're getting the URLFetcher response with old data :)

     return;
 
   LogMetric(DEVICES_REQUESTED);
@@ -502,25 +540,6 @@ void ChromeToMobileService::RequestDeviceSearch() {
   search_request_->Start();
 }
 
-void ChromeToMobileService::HandleAccountInfoResponse() {
-  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
-
-  std::string data;
-  account_info_request_->GetResponseAsString(&data);
-  account_info_request_.reset();
-
-  ListValue* services = NULL;
-  DictionaryValue* dictionary = NULL;
-  scoped_ptr<Value> json(base::JSONReader::Read(data));
-  StringValue cloud_print_service(kCloudPrintSerivceValue);
-  if (json.get() && json->GetAsDictionary(&dictionary) && dictionary &&
-      dictionary->GetList(kAccountServicesKey, &services) && services &&
-      services->Find(cloud_print_service) != services->end()) {
-    cloud_print_accessible_ = true;
-    RequestMobileListUpdate();
-  }
-}
-
 void ChromeToMobileService::HandleSearchResponse() {
   DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
 
@@ -555,11 +574,8 @@ void ChromeToMobileService::HandleSearchResponse() {
       }
     }
 
-    // Update the mobile list and timestamp in prefs.
-    PrefService* prefs = profile_->GetPrefs();
-    prefs->Set(prefs::kChromeToMobileDeviceList, mobiles);
-    prefs->SetInt64(prefs::kChromeToMobileTimestamp,
-                    base::TimeTicks::Now().ToInternalValue());
+    // Update the cached mobile device list in profile prefs.
+    profile_->GetPrefs()->Set(prefs::kChromeToMobileDeviceList, mobiles);
 
     if (HasMobiles())
       LogMetric(DEVICES_AVAILABLE);