Skip enhanced bookmarks cluster requests if it's not being used.

chrome/browser/enhanced_bookmarks/chrome_bookmark_server_cluster_service.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/enhanced_bookmarks/chrome_bookmark_server_cluster_service.h"
 
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "components/enhanced_bookmarks/enhanced_bookmark_model.h"
 
 namespace enhanced_bookmarks {
@@ skipping 15 matching lines @@
       sync_service_(sync_service) {
   if (sync_service_)
     sync_service_->AddObserver(this);
 }
 
 ChromeBookmarkServerClusterService::~ChromeBookmarkServerClusterService() {
   if (sync_service_)
     sync_service_->RemoveObserver(this);
 }
 
+void ChromeBookmarkServerClusterService::AddObserver(
+    BookmarkServerServiceObserver* observer) {
+  BookmarkServerClusterService::AddObserver(observer);
+  if (sync_refresh_skipped_) {
+    sync_refresh_skipped_ = false;
+    TriggerTokenRequest(false);
+  }
+}
+
 void ChromeBookmarkServerClusterService::OnStateChanged() {
   // Do nothing.
 }
 
 void ChromeBookmarkServerClusterService::OnSyncCycleCompleted() {
   // The stars cluster API relies on the information in chrome-sync. Sending a
   // cluster request immediately after a bookmark is changed from the bookmark
   // observer notification will yield the wrong results. The request must be
   // delayed until the sync cycle has completed.
-  // TODO(noyau): This might happen too often, need a way to coalesce and delay
-  // the notifications.
-  if (model_->loaded())
-    TriggerTokenRequest(false);
+  if (model_->loaded()) {

[danduong, 2014/10/20 18:45:38]

Did we want to move this to the end of Extensive Change rather than sync cycle
completion?

Also, considering updating the comment to reflect that we want to avoid calling
this frequently.

[Kibeom Kim (inactive), 2014/10/20 20:05:47]

noyau@: do you have any comment about this?

[noyau (Ping after 24h), 2014/10/20 22:19:54]

How would you know that an extensive change was actually triggered by sync and
not by something else?

The problem is that we lack a good notification. This code wants to know when
sync finished and there was bookmark updates. AFAIK there is no good API to do
this. OnSyncCycleCompleted() fires for any sync data and there is no guarantees
that an extensive change was done by sync.

+    if (MightHaveObservers()) {
+      TriggerTokenRequest(false);
+      sync_refresh_skipped_ = false;
+    } else {
+      sync_refresh_skipped_ = true;
+    }
+  }
 }
 
 }  // namespace enhanced_bookmarks