sync: Re-implement getAllNodes WebUI function

chrome/browser/sync/profile_sync_service.cc

 // Copyright (c) 2012 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/sync/profile_sync_service.h"
 
 #include <cstddef>
 #include <map>
 #include <set>
 #include <utility>
@@ skipping 64 matching lines @@
 #include "grit/generated_resources.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "sync/api/sync_error.h"
 #include "sync/internal_api/public/configure_reason.h"
 #include "sync/internal_api/public/http_bridge_network_resources.h"
 #include "sync/internal_api/public/network_resources.h"
 #include "sync/internal_api/public/sync_encryption_handler.h"
 #include "sync/internal_api/public/util/experiments.h"
 #include "sync/internal_api/public/util/sync_string_conversions.h"
-#include "sync/js/js_arg_list.h"
 #include "sync/js/js_event_details.h"
 #include "sync/util/cryptographer.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/time_format.h"
 
 #if defined(ENABLE_MANAGED_USERS)
 #include "chrome/browser/managed_mode/managed_user_constants.h"
 #endif
 
 #if defined(OS_ANDROID)
@@ skipping 1976 matching lines @@
 }
 
 void ProfileSyncService::RemoveProtocolEventObserver(
     browser_sync::ProtocolEventObserver* observer) {
   protocol_event_observers_.RemoveObserver(observer);
   if (backend_ && !protocol_event_observers_.might_have_observers()) {
     backend_->DisableProtocolEventForwarding();
   }
 }
 
+namespace {
+
+class GetAllNodesRequestHelper
+    : public base::RefCountedThreadSafe<GetAllNodesRequestHelper> {
+ public:
+  GetAllNodesRequestHelper(
+      syncer::ModelTypeSet requested_types,
+      base::Callback<void(scoped_ptr<base::ListValue>)> callback);

[Nicolas Zea, 2014/04/04 23:01:27]

pass callbacks by const ref, here and elsewhere

[rlarocque, 2014/04/05 00:07:27]

Done.

+
+  void OnReceivedNodesForTypes(

[Nicolas Zea, 2014/04/04 23:01:27]

Comment for method. In particular, what is |types|.

[rlarocque, 2014/04/05 00:07:27]

Done.

+      std::vector<syncer::ModelType> types,

[Nicolas Zea, 2014/04/04 23:01:27]

pass types by const ref (and maybe scoped_node_lists too?)

[rlarocque, 2014/04/05 00:07:27]

The scoped_node_list is not const for a reason.  This function call transfers
ownership of the ScopedVector.

+      ScopedVector<base::ListValue> scoped_node_lists);
+
+ private:
+  friend class base::RefCountedThreadSafe<GetAllNodesRequestHelper>;
+  ~GetAllNodesRequestHelper();

[Nicolas Zea, 2014/04/04 23:01:27]

virtual

[rlarocque, 2014/04/05 00:07:27]

Done.

+
+  scoped_ptr<base::ListValue> result_accumulator_;
+
+  syncer::ModelTypeSet awaiting_types_;
+  base::Callback<void(scoped_ptr<base::ListValue>)> callback_;
+};
+
+GetAllNodesRequestHelper::GetAllNodesRequestHelper(
+    syncer::ModelTypeSet requested_types,
+    base::Callback<void(scoped_ptr<base::ListValue>)> callback)
+  : result_accumulator_(new base::ListValue()),

[Nicolas Zea, 2014/04/04 23:01:27]

indent by two more spaces

[rlarocque, 2014/04/05 00:07:27]

Done.

+    awaiting_types_(requested_types),
+    callback_(callback) {}
+
+GetAllNodesRequestHelper::~GetAllNodesRequestHelper() {
+  if (!awaiting_types_.Empty()) {
+    DLOG(WARNING)
+        << "GetAllNodesRequest deleted before request was fulfilled.  "
+        << "Missing types are: " << ModelTypeSetToString(awaiting_types_);
+  }
+}
+
+void GetAllNodesRequestHelper::OnReceivedNodesForTypes(
+    std::vector<syncer::ModelType> types,
+    ScopedVector<base::ListValue> scoped_node_lists) {
+  DCHECK_EQ(types.size(), scoped_node_lists.size());
+
+  // Take unsafe ownership of the node list.
+  std::vector<base::ListValue*> node_lists;
+  scoped_node_lists.release(&node_lists);
+
+  for (size_t i = 0; i < node_lists.size() && i < types.size(); ++i) {
+    const ModelType type = types[i];
+    base::ListValue* node_list = node_lists[i];
+
+    // Add these results to our list.
+    scoped_ptr<base::DictionaryValue> type_dict(new base::DictionaryValue());
+    type_dict->SetString("type", ModelTypeToString(type));
+    type_dict->Set("nodes", node_list);
+    result_accumulator_->Append(type_dict.release());
+
+    // Remember that this part of the request is satisfied.
+    awaiting_types_.Remove(type);
+  }
+
+  if (awaiting_types_.Empty()) {
+    callback_.Run(result_accumulator_.Pass());
+    callback_ = base::Callback<void(scoped_ptr<base::ListValue>)>();

[Nicolas Zea, 2014/04/04 23:01:27]

callback_.Reset()?

[rlarocque, 2014/04/05 00:07:27]

Thanks.  I was looking for that method, but I couldn't remember its name.

+  }
+}
+
+}  // namespace
+
+void ProfileSyncService::GetAllNodes(
+    base::Callback<void(scoped_ptr<base::ListValue>)> callback) {
+  // TODO(rlarocque): Should be GetRegisteredDirectoryTypes.
+  const ModelTypeSet directory_types = GetRegisteredDataTypes();
+  scoped_refptr<GetAllNodesRequestHelper> helper =

[Nicolas Zea, 2014/04/04 23:01:27]

Do we want to support multiple oustanding requests at once? Perhaps have the PSS
own a helper_ object, so it can track whether an outstanding request is being
fulfilled or not?

[rlarocque, 2014/04/05 00:07:27]

It does support multiple outstanding requests.  The GetAllNodesRequestHelper is
that helper object.

The GetAllNodesRequestHelper is RefCountedThreadSafe.  The idea is that we can
split up the node fetching into several different callbacks, and each of them
will increment the helper object's refcout as long as it exists.  Once those
requests are issued, the ProfileSyncService lets go of it.  It will continue to
exist until all the callbacks that have bound to it return.  When that happens,
it will invoke its callback then get deleted as its last callback goes out of
scope.  

[Nicolas Zea, 2014/04/05 00:30:29]

I realize it wasn't clear, but I was actually arguing against allowing multiple
outstanding requests. But I guess the biggest risk would be the user refreshing
a bunch of times and therefore triggering a ton of work, which isn't a big issue
(and I suspect involves refreshing the page in addition to hitting the refresh
button).

[rlarocque, 2014/04/05 00:42:05]

Fortunately, the UI greys out the refresh button while a request is in progress.

+      new GetAllNodesRequestHelper(directory_types, callback);
+
+  if (!backend_initialized_) {
+    // If there's no backend available to fulfill the request, handle it here.
+    ScopedVector<base::ListValue> empty_results;
+    std::vector<ModelType> type_vector;
+    for (ModelTypeSet::Iterator it = directory_types.First();
+         it.Good(); it.Inc()) {
+      type_vector.push_back(it.Get());
+      empty_results.push_back(new base::ListValue());
+    }
+    helper->OnReceivedNodesForTypes(type_vector, empty_results.Pass());
+  } else {
+    backend_->GetAllNodesForTypes(
+        directory_types,
+        base::Bind(&GetAllNodesRequestHelper::OnReceivedNodesForTypes, helper));
+  }
+}
+
 bool ProfileSyncService::HasObserver(
     ProfileSyncServiceBase::Observer* observer) const {
   return observers_.HasObserver(observer);
 }
 
 base::WeakPtr<syncer::JsController> ProfileSyncService::GetJsController() {
   return sync_js_controller_.AsWeakPtr();
 }
 
 void ProfileSyncService::SyncEvent(SyncEventCodes code) {
@@ skipping 123 matching lines @@
   status.last_get_token_error = last_get_token_error_;
   if (request_access_token_retry_timer_.IsRunning())
     status.next_token_request_time = next_token_request_time_;
   return status;
 }
 
 void ProfileSyncService::OverrideNetworkResourcesForTest(
     scoped_ptr<syncer::NetworkResources> network_resources) {
   network_resources_ = network_resources.Pass();
 }