sync: Route local sessions events to SessionsSyncManager

chrome/browser/sync/sessions2/sessions_sync_manager.h

 // Copyright 2013 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.
 
 #ifndef CHROME_BROWSER_SYNC_SESSIONS2_SESSIONS_SYNC_MANAGER_H_
 #define CHROME_BROWSER_SYNC_SESSIONS2_SESSIONS_SYNC_MANAGER_H_
 
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/basictypes.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/scoped_vector.h"
+#include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
 #include "chrome/browser/sessions/session_id.h"
 #include "chrome/browser/sessions/session_types.h"
 #include "chrome/browser/sync/glue/device_info.h"
 #include "chrome/browser/sync/glue/favicon_cache.h"
 #include "chrome/browser/sync/glue/synced_session.h"
 #include "chrome/browser/sync/glue/synced_session_tracker.h"
 #include "chrome/browser/sync/open_tabs_ui_delegate.h"
 #include "chrome/browser/sync/sessions2/tab_node_pool2.h"
 #include "chrome/browser/sync/sync_prefs.h"
@@ skipping 12 matching lines @@
 class SessionWindow;
 class TabNavigation;
 }  // namespace sync_pb
 
 namespace browser_sync {
 
 class DataTypeErrorHandler;
 class SyncedTabDelegate;
 class SyncedWindowDelegate;
 
+// An interface defining the ways in which local open tab events can interact
+// with session sync.  All local tab events flow to sync via this interface.
+// In that way it is analogous to sync changes flowing to the local model
+// via ProcessSyncChanges, just with a more granular breakdown.
+class LocalSessionEventHandler {
+ public:
+  // A local navigation event took place that affects the synced session
+  // for this instance of Chrome.
+  virtual void OnLocalTabModified(SyncedTabDelegate* modified_tab) = 0;
+
+  // When a Browser window is opened, we want to know so we can make sure our
+  // bookkeeping of open windows / sessions on this device is up-to-date.
+  virtual void OnBrowserOpened() = 0;

[Nicolas Zea, 2013/11/21 02:34:52]

Is this necessary? We always reassociate windows anyways; providing this kind of
implies that OnLocalTabModified isn't itself looking at windows.

Perhaps just have OnLocalTabModified support a NULL tab delegate (and comment
that that only looks at the windows)?

[tim (not reviewing), 2013/11/21 21:38:59]

I could maybe change it to OnBrowserModified(SyncedTabDelegate* modified_tab).
On the other hand, I would argue that this interface is meant to only define
"what sorts of events are interesting to tab sync" versus what it does in
response to different events. 

There are some similarities between the two implementations but OLTM does
additional stuff and it's kind of nice to not have an extra state to handle. I
also think the current name is nice because it makes it clear we only handle one
tab at a time and the browser opened event is separate (null |modified_tab|
doesn't imply a "multiple tab" browser opened event). A comment something like
"A Browser was opened [x]or a single tab was modified" might be what we'd want.

If we forsee more events in the future we want to react to where the
implementation will always be identical and we won't be tempted to add
yet-another state (like handling the NULL parameter), I could be swayed. If we
reduced the 3 to one single callback I might agree more, but we're likely going
to have separate functions with different arguments anyway (favicons).

 

[Nicolas Zea, 2013/11/22 23:34:51]

How about two methods, with the second method being named OnBrowserModified, and
a comment that only one of these methods should be called for a single event.

[tim (not reviewing), 2013/11/25 18:12:08]

As discussed, went ahead and removed OnBrowserOpened. 

+
+  // A local navigation occurred that triggered updates to favicon data for
+  // each URL in |updated_page_urls|.  This is routed through Sessions Sync so
+  // that we can filter (exclude) favicon updates for pages that aren't
+  // currently part of the set of local open tabs, and pass relevant updates
+  // on to FaviconCache for out-of-band favicon syncing.
+  virtual void OnFaviconPageUrlsUpdated(
+      const std::set<GURL>& updated_page_urls) = 0;
+};
+
 // Contains all logic for associating the Chrome sessions model and
 // the sync sessions model.
 class SessionsSyncManager : public syncer::SyncableService,
-                            public OpenTabsUIDelegate {
+                            public OpenTabsUIDelegate,
+                            public LocalSessionEventHandler {
  public:
   // Isolates SessionsSyncManager from having to depend on sync internals.
   class SyncInternalApiDelegate {
    public:
+    virtual ~SyncInternalApiDelegate() {}

[Nicolas Zea, 2013/11/21 02:34:52]

nit: newline after

[tim (not reviewing), 2013/11/21 21:38:59]

Done.

     // Returns sync's representation of the local device info.
     // Return value is an empty scoped_ptr if the device info is unavailable.
     virtual scoped_ptr<DeviceInfo> GetLocalDeviceInfo() const = 0;
 
     // Used for creation of the machine tag for this local session.
     virtual std::string GetLocalSyncCacheGUID() const = 0;
   };
 
+  // The LocalEventRouter is responsible for hooking itself up to various
+  // notification sources in the browser process and forwarding relevant
+  // events to a handler as defined in the LocalSessionEventHandler contract.
+  class LocalEventRouter {
+   public:
+    virtual ~LocalEventRouter() {}
+    virtual void StartRoutingTo(LocalSessionEventHandler* handler) = 0;
+    virtual void Stop() = 0;
+  };
+
   SessionsSyncManager(Profile* profile,
-                      SyncInternalApiDelegate* delegate);
+                      SyncInternalApiDelegate* delegate,
+                      scoped_ptr<LocalEventRouter> router);
   virtual ~SessionsSyncManager();
 
-  // A local navigation event took place that affects the synced session
-  // for this instance of Chrome.
-  void OnLocalTabModified(const SyncedTabDelegate& modified_tab,
-                          syncer::SyncError* error);
-
-  // When a Browser window is opened, we want to know so we can make sure our
-  // bookkeeping of open windows / sessions on this device is up-to-date.
-  void OnBrowserOpened();
-
-  // A local navigation occurred that triggered updates to favicon data for
-  // each URL in |updated_page_urls|.  This is routed through Sessions Sync so
-  // that we can filter (exclude) favicon updates for pages that aren't
-  // currently part of the set of local open tabs, and pass relevant updates
-  // on to FaviconCache for out-of-band favicon syncing.
-  void ForwardRelevantFaviconUpdatesToFaviconCache(
-      const std::set<GURL>& updated_favicon_page_urls);
-
-  // Returns the tag used to uniquely identify this machine's session in the
-  // sync model.
-  const std::string& current_machine_tag() const {
-    DCHECK(!current_machine_tag_.empty());
-    return current_machine_tag_;
-  }
-
   // syncer::SyncableService implementation.
   virtual syncer::SyncMergeResult MergeDataAndStartSyncing(
       syncer::ModelType type,
       const syncer::SyncDataList& initial_sync_data,
       scoped_ptr<syncer::SyncChangeProcessor> sync_processor,
       scoped_ptr<syncer::SyncErrorFactory> error_handler) OVERRIDE;
   virtual void StopSyncing(syncer::ModelType type) OVERRIDE;
   virtual syncer::SyncDataList GetAllSyncData(
       syncer::ModelType type) const OVERRIDE;
   virtual syncer::SyncError ProcessSyncChanges(
       const tracked_objects::Location& from_here,
       const syncer::SyncChangeList& change_list) OVERRIDE;
 
-  // Return the virtual URL of the current tab, even if it's pending.
-  static GURL GetCurrentVirtualURL(const SyncedTabDelegate& tab_delegate);
-
-  // Return the favicon url of the current tab, even if it's pending.
-  static GURL GetCurrentFaviconURL(const SyncedTabDelegate& tab_delegate);
-
-  FaviconCache* GetFaviconCache();
-
   // OpenTabsUIDelegate implementation.
   virtual bool GetSyncedFaviconForPageURL(
       const std::string& pageurl,
       scoped_refptr<base::RefCountedMemory>* favicon_png) const OVERRIDE;
   virtual bool GetAllForeignSessions(
       std::vector<const SyncedSession*>* sessions) OVERRIDE;
   virtual bool GetForeignSession(
       const std::string& tag,
       std::vector<const SessionWindow*>* windows) OVERRIDE;
   virtual bool GetForeignTab(const std::string& tag,
                              const SessionID::id_type tab_id,
                              const SessionTab** tab) OVERRIDE;
   virtual void DeleteForeignSession(const std::string& tag) OVERRIDE;
 
+  // LocalSessionEventHandler implementation.
+  virtual void OnLocalTabModified(SyncedTabDelegate* modified_tab) OVERRIDE;
+  virtual void OnBrowserOpened() OVERRIDE;
+  virtual void OnFaviconPageUrlsUpdated(
+      const std::set<GURL>& updated_favicon_page_urls) OVERRIDE;
+
+  // Returns the tag used to uniquely identify this machine's session in the
+  // sync model.
+  const std::string& current_machine_tag() const {
+    DCHECK(!current_machine_tag_.empty());
+    return current_machine_tag_;
+  }
+
+  // Return the virtual URL of the current tab, even if it's pending.
+  static GURL GetCurrentVirtualURL(const SyncedTabDelegate& tab_delegate);
+
+  // Return the favicon url of the current tab, even if it's pending.
+  static GURL GetCurrentFaviconURL(const SyncedTabDelegate& tab_delegate);
+
+  FaviconCache* GetFaviconCache();
+
  private:
   // Keep all the links to local tab data in one place. A tab_node_id and tab
   // must be passed at creation. The tab_node_id is not mutable, although
   // all other fields are.
   class TabLink {
    public:
     TabLink(int tab_node_id, const SyncedTabDelegate* tab)
       : tab_node_id_(tab_node_id),
         tab_(tab) {}
 
@@ skipping 164 matching lines @@
   // Unique client tag.
   std::string current_machine_tag_;
 
   // User-visible machine name.
   std::string current_session_name_;
 
   // SyncID for the sync node containing all the window information for this
   // client.
   int local_session_header_node_id_;
 
+  scoped_ptr<LocalEventRouter> local_event_router_;
+
   DISALLOW_COPY_AND_ASSIGN(SessionsSyncManager);
 };
 
 }  // namespace browser_sync
 
 #endif  // CHROME_BROWSER_SYNC_SESSIONS2_SESSIONS_SYNC_MANAGER_H_