[USS] Move GetAllNodes from backend to controller

components/browser_sync/browser/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 "components/browser_sync/browser/profile_sync_service.h"
 
 #include <stddef.h>
 
 #include <cstddef>
 #include <map>
 #include <utility>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/feature_list.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
@@ skipping 21 matching lines @@
 #include "components/sync/core/shared_model_type_processor.h"
 #include "components/sync/core/shutdown_reason.h"
 #include "components/sync/core/sync_encryption_handler.h"
 #include "components/sync/device_info/device_info.h"
 #include "components/sync/device_info/device_info_service.h"
 #include "components/sync/device_info/device_info_sync_service.h"
 #include "components/sync/device_info/device_info_tracker.h"
 #include "components/sync/driver/backend_migrator.h"
 #include "components/sync/driver/change_processor.h"
 #include "components/sync/driver/data_type_controller.h"
+#include "components/sync/driver/directory_data_type_controller.h"
 #include "components/sync/driver/glue/chrome_report_unrecoverable_error.h"
 #include "components/sync/driver/glue/sync_backend_host.h"
 #include "components/sync/driver/glue/sync_backend_host_impl.h"
 #include "components/sync/driver/pref_names.h"
 #include "components/sync/driver/signin_manager_wrapper.h"
 #include "components/sync/driver/sync_api_component_factory.h"
 #include "components/sync/driver/sync_client.h"
 #include "components/sync/driver/sync_driver_switches.h"
 #include "components/sync/driver/sync_error_controller.h"
 #include "components/sync/driver/sync_stopped_reporter.h"
 #include "components/sync/driver/sync_type_preference_provider.h"
 #include "components/sync/driver/sync_util.h"
 #include "components/sync/driver/system_encryptor.h"
 #include "components/sync/driver/user_selectable_sync_type.h"
 #include "components/sync/engine/cycle/model_neutral_state.h"
 #include "components/sync/engine/cycle/type_debug_info_observer.h"
 #include "components/sync/engine/sync_string_conversions.h"
 #include "components/sync/js/js_event_details.h"
 #include "components/sync/protocol/sync.pb.h"
 #include "components/sync/syncable/directory.h"
+#include "components/sync/syncable/syncable_read_transaction.h"
 #include "components/sync_sessions/favicon_cache.h"
 #include "components/sync_sessions/session_data_type_controller.h"
 #include "components/sync_sessions/sessions_sync_manager.h"
 #include "components/sync_sessions/sync_sessions_client.h"
 #include "components/syncable_prefs/pref_service_syncable.h"
 #include "components/version_info/version_info_values.h"
 #include "net/cookies/cookie_monster.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/time_format.h"
@@ skipping 2098 matching lines @@
 
 namespace {
 
 class GetAllNodesRequestHelper
     : public base::RefCountedThreadSafe<GetAllNodesRequestHelper> {
  public:
   GetAllNodesRequestHelper(
       syncer::ModelTypeSet requested_types,
       const base::Callback<void(std::unique_ptr<base::ListValue>)>& callback);
 
-  void OnReceivedNodesForTypes(
-      const std::vector<syncer::ModelType>& types,
-      ScopedVector<base::ListValue> scoped_node_lists);
+  void OnReceivedNodesForType(const syncer::ModelType type,
+                              std::unique_ptr<base::ListValue> node_list);
 
  private:
   friend class base::RefCountedThreadSafe<GetAllNodesRequestHelper>;
   virtual ~GetAllNodesRequestHelper();
 
   std::unique_ptr<base::ListValue> result_accumulator_;
 
   syncer::ModelTypeSet awaiting_types_;
   base::Callback<void(std::unique_ptr<base::ListValue>)> callback_;
 };
 
 GetAllNodesRequestHelper::GetAllNodesRequestHelper(
     syncer::ModelTypeSet requested_types,
     const base::Callback<void(std::unique_ptr<base::ListValue>)>& callback)
     : result_accumulator_(new base::ListValue()),
       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_);
   }
 }
 
-// Called when the set of nodes for a type or set of types has been returned.
-//
-// The nodes for several types can be returned at the same time by specifying
-// their types in the |types| array, and putting their results at the
-// correspnding indices in the |scoped_node_lists|.
-void GetAllNodesRequestHelper::OnReceivedNodesForTypes(
-    const std::vector<syncer::ModelType>& types,
-    ScopedVector<base::ListValue> scoped_node_lists) {
-  DCHECK_EQ(types.size(), scoped_node_lists.size());
+// Called when the set of nodes for a type has been returned.
+// Only return one type of nodes each time.
+void GetAllNodesRequestHelper::OnReceivedNodesForType(
+    const syncer::ModelType type,
+    std::unique_ptr<base::ListValue> node_list) {
+  // Add these results to our list.
+  std::unique_ptr<base::DictionaryValue> type_dict(new base::DictionaryValue());
+  type_dict->SetString("type", ModelTypeToString(type));
+  type_dict->Set("nodes", std::move(node_list));
+  result_accumulator_->Append(std::move(type_dict));
 
-  // 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.
-    std::unique_ptr<base::DictionaryValue> type_dict(
-        new base::DictionaryValue());
-    type_dict->SetString("type", ModelTypeToString(type));
-    type_dict->Set("nodes", node_list);
-    result_accumulator_->Append(std::move(type_dict));
-
-    // Remember that this part of the request is satisfied.
-    awaiting_types_.Remove(type);
-  }
+  // Remember that this part of the request is satisfied.
+  awaiting_types_.Remove(type);
 
   if (awaiting_types_.Empty()) {
     callback_.Run(std::move(result_accumulator_));
     callback_.Reset();
   }
 }
 
 }  // namespace
 
 void ProfileSyncService::GetAllNodes(
     const base::Callback<void(std::unique_ptr<base::ListValue>)>& callback) {
   // TODO(stanisc): crbug.com/328606: Make this work for USS datatypes.
   ModelTypeSet all_types = GetRegisteredDataTypes();
   all_types.PutAll(syncer::ControlTypes());
   scoped_refptr<GetAllNodesRequestHelper> helper =
       new GetAllNodesRequestHelper(all_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 = all_types.First(); it.Good(); it.Inc()) {
-      type_vector.push_back(it.Get());
-      empty_results.push_back(new base::ListValue());
+      helper->OnReceivedNodesForType(it.Get(),
+                                     base::WrapUnique(new base::ListValue()));

[maxbogue, 2016/08/29 21:21:16]

nit: in this case you could just use base::MakeUnique<base::ListValue>().

[Gang Wu, 2016/08/30 20:56:01]

Done.

     }
-    helper->OnReceivedNodesForTypes(type_vector, std::move(empty_results));
   } else {
-    backend_->GetAllNodesForTypes(
+    GetAllNodesForTypes(

[maxbogue, 2016/08/29 21:21:16]

I'm all for modularity, but GetAllNodesForTypes doesn't have any use outside
this function. I'd prefer you just make the !backend_initialized_ case return at
the end, and then insert the GetAllNodesForTypes code here.

[Gang Wu, 2016/08/30 20:56:01]

Done.

         all_types,
-        base::Bind(&GetAllNodesRequestHelper::OnReceivedNodesForTypes, helper));
+        base::Bind(&GetAllNodesRequestHelper::OnReceivedNodesForType, helper));
   }
 }
 
 bool ProfileSyncService::HasObserver(
     const sync_driver::SyncServiceObserver* observer) const {
   return observers_.HasObserver(observer);
 }
 
 base::WeakPtr<syncer::JsController> ProfileSyncService::GetJsController() {
   return sync_js_controller_.AsWeakPtr();
@@ skipping 234 matching lines @@
   if (--outstanding_setup_in_progress_handles_ != 0)
     return;
 
   DCHECK(startup_controller_->IsSetupInProgress());
   startup_controller_->SetSetupInProgress(false);
 
   if (IsBackendInitialized())
     ReconfigureDatatypeManager();
   NotifyObservers();
 }
+
+void ProfileSyncService::GetAllNodesForTypes(
+    syncer::ModelTypeSet types,
+    base::Callback<void(const syncer::ModelType,
+                        std::unique_ptr<base::ListValue>)> callback) {
+  for (syncer::ModelTypeSet::Iterator it = types.First(); it.Good(); it.Inc()) {
+    DataTypeController::TypeMap::const_iterator iter =

[maxbogue, 2016/08/29 21:21:16]

This is really the sort of place it's great to use auto. Or const auto or
whichever works here. Also I'd call it "dtc_iter" to distinguish from "it".

[Gang Wu, 2016/08/30 20:56:01]

Done.

+        data_type_controllers_.find(it.Get());
+    if (iter != data_type_controllers_.end()) {
+      iter->second->GetAllNodes(callback);
+    } else {
+      // Control Types
+      callback.Run(it.Get(),
+                   sync_driver::DirectoryDataTypeController::
+                       GetAllNodesForTypeFromDirectory(
+                           it.Get(), GetUserShare()->directory.get()));
+    }
+  }
+}