[Sync] Componentize synced_tab_delegate

chrome/browser/sync/glue/synced_session_tracker.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/glue/synced_session_tracker.h"
 
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "base/strings/utf_string_conversions.h"
-#include "chrome/browser/sync/glue/synced_session_util.h"
+#include "components/sync_sessions/sync_sessions_client.h"
 
 namespace browser_sync {
 
-SyncedSessionTracker::SyncedSessionTracker() {
+namespace {
+
+// Helper for iterating through all tabs within a window, and all navigations
+// within a tab, to find if there's a valid syncable url.
+bool ShouldSyncSessionWindow(
+    sync_sessions::SyncSessionsClient* sessions_client,
+    const sessions::SessionWindow& window) {
+  for (auto* tab : window.tabs) {

[pavely, 2015/10/16 21:55:20]

Optional comment:
I think in both "for" loops you could use "const auto&"
Also to understand this code I needed to look up types of tabs and navigations.
Maybe it would make sense to spell out types inside "for" loops (I'm not sure).

[Bernhard Bauer, 2015/10/19 08:20:57]

+1 for spelling out types.

[Nicolas Zea, 2015/10/20 23:14:41]

Done.

[Nicolas Zea, 2015/10/20 23:14:41]

Acknowledged.

+    for (auto&& navigation : tab->navigations) {
+      if (sessions_client->ShouldSyncURL(navigation.virtual_url())) {
+        return true;
+      }
+    }
+  }
+  return false;
 }
 
+}  // namespace
+
+SyncedSessionTracker::SyncedSessionTracker(
+    sync_sessions::SyncSessionsClient* sessions_client)
+    : sessions_client_(sessions_client) {}
+
 SyncedSessionTracker::~SyncedSessionTracker() {
   Clear();
 }
 
 void SyncedSessionTracker::SetLocalSessionTag(
     const std::string& local_session_tag) {
   local_session_tag_ = local_session_tag;
 }
 
 bool SyncedSessionTracker::LookupAllForeignSessions(
     std::vector<const sync_driver::SyncedSession*>* sessions) const {
   DCHECK(sessions);
   sessions->clear();
   // Fill vector of sessions from our synced session map.
   for (SyncedSessionMap::const_iterator i =
     synced_session_map_.begin(); i != synced_session_map_.end(); ++i) {
     // Only include foreign sessions with open tabs.
     sync_driver::SyncedSession* foreign_session = i->second;
     if (i->first != local_session_tag_ && !foreign_session->windows.empty()) {
       bool found_tabs = false;
       for (sync_driver::SyncedSession::SyncedWindowMap::const_iterator iter =
                foreign_session->windows.begin();
            iter != foreign_session->windows.end(); ++iter) {
-        if (ShouldSyncSessionWindow(*(iter->second))) {
+        if (ShouldSyncSessionWindow(sessions_client_, *(iter->second))) {
           found_tabs = true;
           break;
         }
       }
       if (found_tabs)
         sessions->push_back(foreign_session);
     }
   }
 
   return !sessions->empty();
@@ skipping 352 matching lines @@
   STLDeleteElements(&unmapped_tabs_);
 
   // Get rid of our Window/Tab maps (does not delete the actual Window/Tabs
   // themselves; they should have all been deleted above).
   synced_window_map_.clear();
   synced_tab_map_.clear();
   local_session_tag_.clear();
 }
 
 }  // namespace browser_sync