sync: Add UI code for per type debug counters

chrome/browser/ui/webui/sync_internals_message_handler.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/ui/webui/sync_internals_message_handler.h"
 
 #include <vector>
 
 #include "base/logging.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/about_sync_util.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_ui.h"
 #include "sync/internal_api/public/events/protocol_event.h"
+#include "sync/internal_api/public/sessions/commit_counters.h"
+#include "sync/internal_api/public/sessions/status_counters.h"
+#include "sync/internal_api/public/sessions/update_counters.h"
 #include "sync/internal_api/public/util/weak_handle.h"
 #include "sync/js/js_event_details.h"
 
 using syncer::JsEventDetails;
 using syncer::ModelTypeSet;
 using syncer::WeakHandle;
 
 SyncInternalsMessageHandler::SyncInternalsMessageHandler()
     : is_registered_(false),
-      weak_ptr_factory_(this) {}
+      is_registered_for_counters_(false),
+      weak_ptr_factory_(this) {
+}
 
 SyncInternalsMessageHandler::~SyncInternalsMessageHandler() {
   if (js_controller_)
     js_controller_->RemoveJsEventHandler(this);
 
   ProfileSyncService* service = GetProfileSyncService();
   if (service && service->HasObserver(this)) {
     service->RemoveObserver(this);
     service->RemoveProtocolEventObserver(this);
   }
+
+  if (service && is_registered_for_counters_) {
+    service->RemoveTypeDebugInfoObserver(this);
+  }
 }
 
 void SyncInternalsMessageHandler::RegisterMessages() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   web_ui()->RegisterMessageCallback(
       "registerForEvents",
       base::Bind(&SyncInternalsMessageHandler::HandleRegisterForEvents,
                  base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
+      "registerForPerTypeCounters",

[maniscalco, 2014/05/23 00:55:00]

I wonder if we could tie into the existing "registerForEvents"
(HandleRegisterForEvents) callback instead of creating a separate one for per
type counters.  If so, we could drop the new flag.  Does anything prohibit us
from having just one register callback?

[rlarocque, 2014/05/27 20:13:26]

It gets complicated.

The problem is that this registration also implicitly sends a request to
immediately issue a series of events that should initialize the newly-registered
listener's state.

If the registration happens as part of the registerForEvents() call in about.js,
we'd have no guarantee that the listeners in types.js will have been setup and
listening by that time.  I think that may be a risk even if the initialization
request and registration were separate, since (I suppose) types.js could request
initialization before registerForEvents() is called, and I'm not sure what the
semantics of that would be.

===

We could probably refactor around this.  I think the eventual "right" way to fix
this would be to implement an abstraction layer between the chrome.send() calls
and responses and the page display logic.  All data received from Chrome would
be stored in a cache.  Listeners could query it explicitly or request
notifications of updates.  sync_log.js follows this pattern.

On the other hand, this is supposed to be a simple, developer-facing UI for
displaying data exported from sync.  It would be nice if we could keep this page
as dumb as possible.  I'm not sure if implementing the technically "right"
solution would be a good idea in this context.

+      base::Bind(&SyncInternalsMessageHandler::HandleRegisterForPerTypeCounters,
+                 base::Unretained(this)));
+
+  web_ui()->RegisterMessageCallback(
       "requestUpdatedAboutInfo",
       base::Bind(&SyncInternalsMessageHandler::HandleRequestUpdatedAboutInfo,
                  base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
       "requestListOfTypes",
       base::Bind(&SyncInternalsMessageHandler::HandleRequestListOfTypes,
                  base::Unretained(this)));
 
   web_ui()->RegisterMessageCallback(
@@ skipping 12 matching lines @@
   ProfileSyncService* service = GetProfileSyncService();
   if (service && !is_registered_) {
     service->AddObserver(this);
     service->AddProtocolEventObserver(this);
     js_controller_ = service->GetJsController();
     js_controller_->AddJsEventHandler(this);
     is_registered_ = true;
   }
 }
 
+void SyncInternalsMessageHandler::HandleRegisterForPerTypeCounters(
+    const base::ListValue* args) {
+  DCHECK(args->empty());
+
+  ProfileSyncService* service = GetProfileSyncService();
+  if (service && !is_registered_for_counters_) {
+    service->AddTypeDebugInfoObserver(this);
+    is_registered_for_counters_ = true;
+  } else {
+    // Re-register to ensure counters get re-emitted.
+    service->RemoveTypeDebugInfoObserver(this);
+    service->AddTypeDebugInfoObserver(this);
+  }
+}
+
 void SyncInternalsMessageHandler::HandleRequestUpdatedAboutInfo(
     const base::ListValue* args) {
   DCHECK(args->empty());
   SendAboutInfo();
 }
 
 void SyncInternalsMessageHandler::HandleRequestListOfTypes(
     const base::ListValue* args) {
   DCHECK(args->empty());
   base::DictionaryValue event_details;
@@ skipping 40 matching lines @@
 void SyncInternalsMessageHandler::OnProtocolEvent(
     const syncer::ProtocolEvent& event) {
   scoped_ptr<base::DictionaryValue> value(
       syncer::ProtocolEvent::ToValue(event));
   web_ui()->CallJavascriptFunction(
       "chrome.sync.dispatchEvent",
       base::StringValue("onProtocolEvent"),
       *value);
 }
 
+void SyncInternalsMessageHandler::OnCommitCountersUpdated(
+    syncer::ModelType type,
+    const syncer::CommitCounters& counters) {
+  scoped_ptr<base::DictionaryValue> details(new base::DictionaryValue());

[maniscalco, 2014/05/23 00:55:00]

Too bad syncer::FooCounters don't have a common base class.  WDYT about using a
function template to reduce duplication?

template <typename T>
scoped_ptr<base::DictionaryValue> CreateDictionaryWithCounters(
    syncer::ModelType type,
    const std::string& description,
    const T& counters) {
  details->SetString("modelType", ModelTypeToString(type));
  details->SetString("counterType", description);
  details->Set("counters", counters.ToValue().release());
}

[rlarocque, 2014/05/27 20:13:26]

I'll admit that seems like a valid use of templates, but I think we have a
pretty strong bias against templates in our code.

I think your proposal still requires three separate listener functions, right? 
What if we pulled the ToValue() call out of proposed template function and into
the type-specific wrapper?  Then CreateDictionaryWithCounters could take a
DictionaryValue instead of a T.

[maniscalco, 2014/05/27 20:31:07]

Yep, you'd still have three listener functions, each with a call to
CreateDictionaryWithCounters.

Pulling the ToValue call up into the listener and passing a DictionaryValue to
the common helper function SGTM.

+  details->SetString("modelType", ModelTypeToString(type));
+  details->SetString("counterType", "commit");
+  details->Set("counters", counters.ToValue().release());
+  web_ui()->CallJavascriptFunction("chrome.sync.dispatchEvent",
+                                   base::StringValue("onCountersUpdated"),
+                                   *details);
+}
+
+void SyncInternalsMessageHandler::OnUpdateCountersUpdated(
+    syncer::ModelType type,
+    const syncer::UpdateCounters& counters) {
+  scoped_ptr<base::DictionaryValue> details(new base::DictionaryValue());
+  details->SetString("modelType", ModelTypeToString(type));
+  details->SetString("counterType", "update");
+  details->Set("counters", counters.ToValue().release());
+  web_ui()->CallJavascriptFunction("chrome.sync.dispatchEvent",
+                                   base::StringValue("onCountersUpdated"),
+                                   *details);
+}
+
+void SyncInternalsMessageHandler::OnStatusCountersUpdated(
+    syncer::ModelType type,
+    const syncer::StatusCounters& counters) {
+  scoped_ptr<base::DictionaryValue> details(new base::DictionaryValue());
+  details->SetString("modelType", ModelTypeToString(type));
+  details->SetString("counterType", "status");
+  details->Set("counters", counters.ToValue().release());
+  web_ui()->CallJavascriptFunction("chrome.sync.dispatchEvent",
+                                   base::StringValue("onCountersUpdated"),
+                                   *details);
+}
+
 void SyncInternalsMessageHandler::HandleJsEvent(
     const std::string& name,
     const JsEventDetails& details) {
   DVLOG(1) << "Handling event: " << name
            << " with details " << details.ToString();
   web_ui()->CallJavascriptFunction("chrome.sync.dispatchEvent",
                                    base::StringValue(name),
                                    details.Get());
 }
 
 void SyncInternalsMessageHandler::SendAboutInfo() {
   scoped_ptr<base::DictionaryValue> value =
       sync_ui_util::ConstructAboutInformation(GetProfileSyncService());
   web_ui()->CallJavascriptFunction(
       "chrome.sync.dispatchEvent",
       base::StringValue("onAboutInfoUpdated"),
       *value);
 }
 
 // Gets the ProfileSyncService of the underlying original profile.
 // May return NULL (e.g., if sync is disabled on the command line).
 ProfileSyncService* SyncInternalsMessageHandler::GetProfileSyncService() {
   Profile* profile = Profile::FromWebUI(web_ui());
   ProfileSyncServiceFactory* factory = ProfileSyncServiceFactory::GetInstance();
   return factory->GetForProfile(profile->GetOriginalProfile());
 }
-