[Sync] SyncEngine refactor part 1: interfaces.

components/sync/engine/sync_engine.h

Index: components/sync/engine/sync_engine.h
diff --git a/components/sync/driver/glue/sync_backend_host.h b/components/sync/engine/sync_engine.h
similarity index 78%
rename from components/sync/driver/glue/sync_backend_host.h
rename to components/sync/engine/sync_engine.h
index ec8ce49679e5f0bec53a3e687a14375fc83a3782..4c9a48865a84354b3ff9420cbff55aa6bf3da0ca 100644
--- a/components/sync/driver/glue/sync_backend_host.h
+++ b/components/sync/engine/sync_engine.h
@@ -2,8 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef COMPONENTS_SYNC_DRIVER_GLUE_SYNC_BACKEND_HOST_H_
-#define COMPONENTS_SYNC_DRIVER_GLUE_SYNC_BACKEND_HOST_H_
+#ifndef COMPONENTS_SYNC_ENGINE_SYNC_ENGINE_H_
+#define COMPONENTS_SYNC_ENGINE_SYNC_ENGINE_H_
 
 #include <memory>
 #include <string>
@@ -14,9 +14,9 @@
 #include "base/single_thread_task_runner.h"
 #include "components/sync/base/model_type.h"
 #include "components/sync/base/weak_handle.h"
-#include "components/sync/driver/backend_data_type_configurer.h"
 #include "components/sync/engine/configure_reason.h"
 #include "components/sync/engine/cycle/sync_cycle_snapshot.h"
+#include "components/sync/engine/model_type_configurer.h"
 #include "components/sync/engine/shutdown_reason.h"
 #include "components/sync/engine/sync_manager.h"
 #include "components/sync/engine/sync_manager_factory.h"
@@ -27,15 +27,15 @@ namespace syncer {
 
 class CancelationSignal;
 class HttpPostProviderFactory;
-class SyncFrontend;
+class SyncEngineHost;
 class SyncManagerFactory;
 class UnrecoverableErrorHandler;
 
 // An API to "host" the top level SyncAPI element.

[skym, 2016/12/02 20:54:55]

Still using the word host. Can you rewrite all of SyncEngine's class comments?
It should be clear from the comments why SyncEngine is a good name.

[maxbogue, 2016/12/02 23:59:28]

Done, PTAL.

 //
 // This class handles dispatch of potentially blocking calls to appropriate
-// threads and ensures that the SyncFrontend is only accessed on the UI loop.
-class SyncBackendHost : public BackendDataTypeConfigurer {
+// threads and ensures that the SyncEngineHost is only accessed on the UI loop.
+class SyncEngine : public ModelTypeConfigurer {
  public:
   typedef SyncStatus Status;

[skym, 2016/12/02 20:54:55]

This typedef is weird.

[maxbogue, 2016/12/02 23:59:28]

Super weird but I ain't touching it in this CL.

   typedef base::Callback<std::unique_ptr<HttpPostProviderFactory>(
@@ -43,17 +43,17 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
       HttpPostProviderFactoryGetter;
 
   // Stubs used by implementing classes.
-  SyncBackendHost();
-  ~SyncBackendHost() override;
+  SyncEngine();
+  ~SyncEngine() override;
 
-  // Called on the frontend's thread to kick off asynchronous initialization.
+  // Called on the host's thread to kick off asynchronous initialization.

[skym, 2016/12/02 20:54:55]

All methods on this class should now be on the host's thread. Why does this
class need 2 step init again?

[maxbogue, 2016/12/02 23:59:28]

I don't know why before; going forward it's how the core will be initialized on
the sync thread so I'm keeping it. Updated the comment though.

   // Optionally deletes the "Sync Data" folder during init in order to make
   // sure we're starting fresh.
   //
   // |saved_nigori_state| is optional nigori state to restore from a previous
   // backend instance. May be null.
   virtual void Initialize(
-      SyncFrontend* frontend,
+      SyncEngineHost* host,
       scoped_refptr<base::SingleThreadTaskRunner> sync_task_runner,
       const WeakHandle<JsEventHandler>& event_handler,
       const GURL& service_url,
@@ -69,21 +69,19 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
       std::unique_ptr<SyncEncryptionHandler::NigoriState>
           saved_nigori_state) = 0;
 
-  // Called on the frontend's thread to trigger a refresh.
+  // Called on the host's thread to trigger a refresh.

[skym, 2016/12/02 20:54:55]

I'd just remove all of these comments. They no longer provide value, right?
Everything should be on the same thread, this should be a single threaded object
now.

[maxbogue, 2016/12/02 23:59:28]

I re-wrote them to be more descriptive instead of just removing them.

   virtual void TriggerRefresh(const ModelTypeSet& types) = 0;
 
-  // Called on the frontend's thread to update SyncCredentials.
+  // Called on the host's thread to update SyncCredentials.

[skym, 2016/12/02 20:54:55]

Same.

[maxbogue, 2016/12/02 23:59:28]

Done.

   virtual void UpdateCredentials(const SyncCredentials& credentials) = 0;
 
   // This starts the SyncerThread running a Syncer object to communicate with
   // sync servers.  Until this is called, no changes will leave or enter this
   // browser from the cloud / sync servers.
-  // Called on |frontend_loop_|.

[skym, 2016/12/02 20:54:55]

Yeah! Like this!

[maxbogue, 2016/12/02 23:59:28]

Acknowledged.

   virtual void StartSyncingWithServer() = 0;
 
-  // Called on |frontend_loop_| to asynchronously set a new passphrase for
-  // encryption. Note that it is an error to call SetEncryptionPassphrase under
-  // the following circumstances:
+  // Asynchronously set a new passphrase for encryption. Note that it is an
+  // error to call SetEncryptionPassphrase under the following circumstances:
   // - An explicit passphrase has already been set
   // - |is_explicit| is true and we have pending keys.
   // When |is_explicit| is false, a couple of things could happen:
@@ -95,21 +93,20 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   virtual void SetEncryptionPassphrase(const std::string& passphrase,
                                        bool is_explicit) = 0;
 
-  // Called on |frontend_loop_| to use the provided passphrase to asynchronously
-  // attempt decryption. Returns false immediately if the passphrase could not
-  // be used to decrypt a locally cached copy of encrypted keys; returns true
-  // otherwise. If new encrypted keys arrive during the asynchronous call,
-  // OnPassphraseRequired may be triggered at a later time. It is an error to
-  // call this when there are no pending keys.
+  // Use the provided passphrase to asynchronously attempt decryption. Returns
+  // false immediately if the passphrase could not be used to decrypt a locally
+  // cached copy of encrypted keys; returns true otherwise. If new encrypted
+  // keys arrive during the asynchronous call, OnPassphraseRequired may be
+  // triggered at a later time. It is an error to call this when there are no
+  // pending keys.
   virtual bool SetDecryptionPassphrase(const std::string& passphrase)
       WARN_UNUSED_RESULT = 0;
 
-  // Called on |frontend_loop_| to kick off shutdown procedure.  Attempts to cut
-  // short any long-lived or blocking sync thread tasks so that the shutdown on
-  // sync thread task that we're about to post won't have to wait very long.
+  // Kick off shutdown procedure. Attempts to cut short any long-lived or
+  // blocking sync thread tasks so that the shutdown on sync thread task that
+  // we're about to post won't have to wait very long.
   virtual void StopSyncingForShutdown() = 0;
 
-  // Called on |frontend_loop_| to kick off shutdown.
   // See the implementation and Core::DoShutdown for details.
   // Must be called *after* StopSyncingForShutdown.
   virtual void Shutdown(ShutdownReason reason) = 0;
@@ -124,7 +121,7 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // is non-empty, then an error was encountered).
   // Returns the set of types that are ready to start without needing any
   // further sync activity.
-  // BackendDataTypeConfigurer implementation.
+  // ModelTypeConfigurer implementation.
   ModelTypeSet ConfigureDataTypes(
       ConfigureReason reason,
       const DataTypeConfigStateMap& config_state_map,
@@ -134,9 +131,9 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   // Turns on encryption of all present and future sync data.
   virtual void EnableEncryptEverything() = 0;
 
-  // Called on |frontend_loop_| to obtain a handle to the UserShare needed for
-  // creating transactions.  Should not be called before we signal
-  // initialization is complete with OnBackendInitialized().
+  // Obtain a handle to the UserShare needed for creating transactions. Should
+  // not be called before we signal initialization is complete with
+  // OnBackendInitialized().
   virtual UserShare* GetUserShare() const = 0;
 
   // Called from any thread to obtain current status information in detailed or
@@ -199,9 +196,9 @@ class SyncBackendHost : public BackendDataTypeConfigurer {
   virtual void OnCookieJarChanged(bool account_mismatch, bool empty_jar) = 0;
 
  private:
-  DISALLOW_COPY_AND_ASSIGN(SyncBackendHost);
+  DISALLOW_COPY_AND_ASSIGN(SyncEngine);
 };
 
 }  // namespace syncer
 
-#endif  // COMPONENTS_SYNC_DRIVER_GLUE_SYNC_BACKEND_HOST_H_
+#endif  // COMPONENTS_SYNC_ENGINE_SYNC_ENGINE_H_