Merge registerForEvents/requestUpdatedAboutInfo sync-internals JS APIs

This CL takes care of a quick clean-up TODO for the linked bug, and no
behavior is changed. registerForEvents() and requestUpdatedAboutInfo()
are two private APIs used by the chrome://sync-internals debugging page
and always called together, so they are now merged into a single API.

Fixed: 1138535
Change-Id: I712959f49bcb89820f369ba5d41c4e50e2203735
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514421
Commit-Queue: Victor Vianna <victorvianna@google.com>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823292}

chrome/browser/ui/webui/sync_internals_message_handler.cc

@@ -98,14 +98,9 @@ void SyncInternalsMessageHandler::RegisterMessages() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   web_ui()->RegisterMessageCallback(
-      syncer::sync_ui_util::kRegisterForEvents,
-      base::BindRepeating(&SyncInternalsMessageHandler::HandleRegisterForEvents,
-                          base::Unretained(this)));
-
-  web_ui()->RegisterMessageCallback(
-      syncer::sync_ui_util::kRequestUpdatedAboutInfo,
+      syncer::sync_ui_util::kRequestDataAndRegisterForUpdates,
       base::BindRepeating(
-          &SyncInternalsMessageHandler::HandleRequestUpdatedAboutInfo,
+          &SyncInternalsMessageHandler::HandleRequestDataAndRegisterForUpdates,
           base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
@@ -165,7 +160,7 @@ void SyncInternalsMessageHandler::RegisterMessages() {
                           base::Unretained(this)));
 }
 
-void SyncInternalsMessageHandler::HandleRegisterForEvents(
+void SyncInternalsMessageHandler::HandleRequestDataAndRegisterForUpdates(
     const ListValue* args) {
   DCHECK(args->empty());
   AllowJavascript();
@@ -188,12 +183,7 @@ void SyncInternalsMessageHandler::HandleRegisterForEvents(
 
     is_registered_ = true;
   }
-}
 
-void SyncInternalsMessageHandler::HandleRequestUpdatedAboutInfo(
-    const ListValue* args) {
-  DCHECK(args->empty());
-  AllowJavascript();
   SendAboutInfoAndEntityCounts();
 }
 

chrome/browser/ui/webui/sync_internals_message_handler.h

@@ -39,13 +39,9 @@ class SyncInternalsMessageHandler : public content::WebUIMessageHandler,
   void OnJavascriptDisallowed() override;
   void RegisterMessages() override;
 
-  // Sets up observers to receive events and forward them to the UI.
-  void HandleRegisterForEvents(const base::ListValue* args);
-
-  // Fires an event to send updated info back to the page.
-  // TODO(crbug.com/1138535): Merge this with HandleRegisterForEvents. Same for
-  // the ios/ counterpart.
-  void HandleRequestUpdatedAboutInfo(const base::ListValue* args);
+  // Fires an event to send updated data to the About page and registers
+  // observers to notify the page upon updates.
+  void HandleRequestDataAndRegisterForUpdates(const base::ListValue* args);
 
   // Fires an event to send the list of types back to the page.
   void HandleRequestListOfTypes(const base::ListValue* args);

chrome/browser/ui/webui/sync_internals_message_handler_unittest.cc

@@ -210,7 +210,7 @@ TEST_F(SyncInternalsMessageHandlerTest, AddRemoveObservers) {
   ListValue empty_list;
 
   EXPECT_EQ(0, test_sync_service()->add_observer_count());
-  handler()->HandleRegisterForEvents(&empty_list);
+  handler()->HandleRequestDataAndRegisterForUpdates(&empty_list);
   EXPECT_EQ(1, test_sync_service()->add_observer_count());
 
   EXPECT_EQ(0, test_sync_service()->remove_observer_count());
@@ -225,7 +225,7 @@ TEST_F(SyncInternalsMessageHandlerTest, AddRemoveObserversDisallowJavascript) {
   ListValue empty_list;
 
   EXPECT_EQ(0, test_sync_service()->add_observer_count());
-  handler()->HandleRegisterForEvents(&empty_list);
+  handler()->HandleRequestDataAndRegisterForUpdates(&empty_list);
   EXPECT_EQ(1, test_sync_service()->add_observer_count());
 
   EXPECT_EQ(0, test_sync_service()->remove_observer_count());
@@ -244,7 +244,7 @@ TEST_F(SyncInternalsMessageHandlerTest, AddRemoveObserversSyncDisabled) {
       profile(), BrowserContextKeyedServiceFactory::TestingFactory());
 
   ListValue empty_list;
-  handler()->HandleRegisterForEvents(&empty_list);
+  handler()->HandleRequestDataAndRegisterForUpdates(&empty_list);
   handler()->DisallowJavascript();
   // Cannot verify observer methods on sync services were not called, because
   // there is no sync service. Rather, we're just making sure the handler hasn't

components/sync/driver/resources/about.js

@@ -222,11 +222,8 @@ cr.define('chrome.sync.about_tab', function() {
       chrome.sync.requestStopClearData();
     });
 
-    // Register to receive a stream of event notifications.
-    chrome.sync.registerForEvents();
-
-    // Request an about info update event to initialize the page.
-    chrome.sync.requestUpdatedAboutInfo();
+    // Request initial data for the page and listen to updates.
+    chrome.sync.requestDataAndRegisterForUpdates();
   }
 
   return {

components/sync/driver/resources/chrome_sync.js

@@ -42,20 +42,14 @@ cr.define('chrome.sync', function() {
   }
 
   /**
-   * Registers to receive a stream of events through
-   * chrome.sync.dispatchEvent().
+   * Requests the sync state, which is sent via onAboutInfoUpdated and
+   * onEntityCountsUpdated events. New events will be emitted whenever the
+   * state changes.
    */
-  function registerForEvents() {
-    chrome.send('registerForEvents');
+  function requestDataAndRegisterForUpdates() {
+    chrome.send('requestDataAndRegisterForUpdates');
   }
 
-  /**
-   * Asks the browser to refresh our snapshot of sync state. Should result
-   * in an onAboutInfoUpdated event being emitted.
-   */
-  function requestUpdatedAboutInfo() {
-    chrome.send('requestUpdatedAboutInfo');
-  }
 
   /**
    * Asks the browser to send us the list of registered types. Should result
@@ -176,8 +170,7 @@ cr.define('chrome.sync', function() {
     events: document.createElement('div'),
     getAllNodes: getAllNodes,
     getAllNodesCallback: getAllNodesCallback,
-    registerForEvents: registerForEvents,
-    requestUpdatedAboutInfo: requestUpdatedAboutInfo,
+    requestDataAndRegisterForUpdates: requestDataAndRegisterForUpdates,
     requestIncludeSpecificsInitialState: requestIncludeSpecificsInitialState,
     requestListOfTypes: requestListOfTypes,
     requestUserEventsVisibility: requestUserEventsVisibility,

components/sync/driver/sync_internals_util.cc

@@ -53,14 +53,14 @@ const char kInvalidationsJS[] = "invalidations.js";
 const char kDispatchEvent[] = "chrome.sync.dispatchEvent";
 const char kGetAllNodes[] = "getAllNodes";
 const char kGetAllNodesCallback[] = "chrome.sync.getAllNodesCallback";
-const char kRegisterForEvents[] = "registerForEvents";
+const char kRequestDataAndRegisterForUpdates[] =
+    "requestDataAndRegisterForUpdates";
 const char kRequestIncludeSpecificsInitialState[] =
     "requestIncludeSpecificsInitialState";
 const char kRequestListOfTypes[] = "requestListOfTypes";
 const char kRequestStart[] = "requestStart";
 const char kRequestStopKeepData[] = "requestStopKeepData";
 const char kRequestStopClearData[] = "requestStopClearData";
-const char kRequestUpdatedAboutInfo[] = "requestUpdatedAboutInfo";
 const char kRequestUserEventsVisibility[] = "requestUserEventsVisibility";
 const char kSetIncludeSpecifics[] = "setIncludeSpecifics";
 const char kTriggerRefresh[] = "triggerRefresh";

components/sync/driver/sync_internals_util.h

@@ -46,13 +46,12 @@ extern const char kInvalidationsJS[];
 extern const char kDispatchEvent[];
 extern const char kGetAllNodes[];
 extern const char kGetAllNodesCallback[];
-extern const char kRegisterForEvents[];
+extern const char kRequestDataAndRegisterForUpdates[];
 extern const char kRequestIncludeSpecificsInitialState[];
 extern const char kRequestListOfTypes[];
 extern const char kRequestStart[];
 extern const char kRequestStopKeepData[];
 extern const char kRequestStopClearData[];
-extern const char kRequestUpdatedAboutInfo[];
 extern const char kRequestUserEventsVisibility[];
 extern const char kSetIncludeSpecifics[];
 extern const char kTriggerRefresh[];

ios/components/webui/sync_internals/sync_internals_message_handler.h

@@ -33,11 +33,9 @@ class SyncInternalsMessageHandler : public web::WebUIIOSMessageHandler,
 
   void RegisterMessages() override;
 
-  // Sets up observers to receive events and forward them to the UI.
-  void HandleRegisterForEvents(const base::ListValue* args);
-
-  // Fires an event to send updated info back to the page.
-  void HandleRequestUpdatedAboutInfo(const base::ListValue* args);
+  // Fires an event to send updated data to the About page and registers
+  // observers to notify the page upon updates.
+  void HandleRequestDataAndRegisterForUpdates(const base::ListValue* args);
 
   // Fires an event to send the list of types back to the page.
   void HandleRequestListOfTypes(const base::ListValue* args);

ios/components/webui/sync_internals/sync_internals_message_handler.mm

@@ -58,14 +58,9 @@ void SyncInternalsMessageHandler::RegisterMessages() {
   DCHECK_CURRENTLY_ON(web::WebThread::UI);
 
   web_ui()->RegisterMessageCallback(
-      syncer::sync_ui_util::kRegisterForEvents,
-      base::BindRepeating(&SyncInternalsMessageHandler::HandleRegisterForEvents,
-                          base::Unretained(this)));
-
-  web_ui()->RegisterMessageCallback(
-      syncer::sync_ui_util::kRequestUpdatedAboutInfo,
+      syncer::sync_ui_util::kRequestDataAndRegisterForUpdates,
       base::BindRepeating(
-          &SyncInternalsMessageHandler::HandleRequestUpdatedAboutInfo,
+          &SyncInternalsMessageHandler::HandleRequestDataAndRegisterForUpdates,
           base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
@@ -114,7 +109,7 @@ void SyncInternalsMessageHandler::RegisterMessages() {
                           base::Unretained(this)));
 }
 
-void SyncInternalsMessageHandler::HandleRegisterForEvents(
+void SyncInternalsMessageHandler::HandleRequestDataAndRegisterForUpdates(
     const base::ListValue* args) {
   DCHECK(args->empty());
 
@@ -129,11 +124,7 @@ void SyncInternalsMessageHandler::HandleRegisterForEvents(
     js_controller_->AddJsEventHandler(this);
     is_registered_ = true;
   }
-}
 
-void SyncInternalsMessageHandler::HandleRequestUpdatedAboutInfo(
-    const base::ListValue* args) {
-  DCHECK(args->empty());
   SendAboutInfoAndEntityCounts();
 }