[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>
@@ skipping 44 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 2099 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,
-      std::vector<std::unique_ptr<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
-// corresponding indices in the |node_lists|.
-void GetAllNodesRequestHelper::OnReceivedNodesForTypes(
-    const std::vector<syncer::ModelType>& types,
-    std::vector<std::unique_ptr<base::ListValue>> node_lists) {
-  DCHECK_EQ(types.size(), 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.

[maxbogue, 2016/08/30 21:47:43]

Perhaps DCHECK at the top of this method that |type| is in |awaiting_types_|?

+  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));
 
-  for (size_t i = 0; i < node_lists.size() && i < types.size(); ++i) {
-    const ModelType type = types[i];
-    std::unique_ptr<base::Value> node_list = std::move(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", std::move(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();
+  ModelTypeSet all_types = GetActiveDataTypes();
   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.
-    std::vector<std::unique_ptr<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(base::MakeUnique<base::ListValue>());
+      helper->OnReceivedNodesForType(it.Get(),
+                                     base::MakeUnique<base::ListValue>());
     }
-    helper->OnReceivedNodesForTypes(type_vector, std::move(empty_results));
   } else {

[maxbogue, 2016/08/30 21:47:44]

I would still prefer putting a return statement at the end of the if and
removing the else. That style is commonly used for handling error cases and
helps emphasize that the below section is the "normal" code path that will be
followed (mostly by it not being indented).

[Gang Wu, 2016/08/30 23:46:06]

Done.

-    backend_->GetAllNodesForTypes(
-        all_types,
-        base::Bind(&GetAllNodesRequestHelper::OnReceivedNodesForTypes, helper));
+    for (syncer::ModelTypeSet::Iterator it = all_types.First(); it.Good();

[maxbogue, 2016/08/30 21:47:43]

Remove "syncer::"; we do using syncer::ModelTypeSet at the top of the file.

[Gang Wu, 2016/08/30 23:46:06]

Done.

+         it.Inc()) {
+      const auto& dtc_iter = data_type_controllers_.find(it.Get());
+      if (dtc_iter != data_type_controllers_.end()) {
+        dtc_iter->second->GetAllNodes(base::Bind(
+            &GetAllNodesRequestHelper::OnReceivedNodesForType, helper));
+      } else {
+        // Control Types
+        helper->OnReceivedNodesForType(
+            it.Get(), sync_driver::DirectoryDataTypeController::
+                          GetAllNodesForTypeFromDirectory(
+                              it.Get(), GetUserShare()->directory.get()));
+      }
+    }
   }
 }
 
 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();
 }