Avoid a proxy tab helper on Android for sync delegation

Prior to this patch, TabContentsSyncedTabDelegate was itself a tab
helper (WebContentsUserData) that was created on all platforms, with the
whole purpose to reuse a partial implementation of
sync_sessions::SyncedTabDelegate.

This was a bit weird because
a) The life cycle diverges across platforms: created lazily on Android,
   early otherwise.
b) On Android there was no real need to register a tab helper.
c) On Android, it's weird to reason about two tab delegates coexisting,
   one of which is a proxy but also overrides some logic.

Instead, let's make TabContentsSyncedTabDelegate a base class that is
*NOT* a WebContentsUserData, and let platform-specific subclasses
decide whether they want to register as WebContentsUserData, as well
as which logic they need to override.

This also removes code that is dead on some platforms, because the
base class is abtract and doesn't implements all methods (as opposed
to a proxy object which cannot be abstract).

Bug: 851905
Change-Id: I305ad85e2392fdd2ce428c0464b7750ac599babf
Reviewed-on: https://chromium-review.googlesource.com/1097405
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567213}

chrome/browser/media/android/remote/remote_media_player_manager.cc

@@ -7,6 +7,7 @@
 #include "chrome/browser/android/tab_android.h"
 #include "chrome/common/chrome_content_client.h"
 #include "content/common/media/media_player_messages_android.h"
+#include "content/public/browser/web_contents.h"
 #include "third_party/blink/public/platform/modules/remoteplayback/web_remote_playback_availability.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/android/java_bitmap.h"

chrome/browser/media/android/router/media_router_android.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/media/router/route_message_observer.h"
 #include "chrome/common/media_router/route_request_result.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/web_contents.h"
 #include "content/public/common/presentation_connection_message.h"
 #include "url/gurl.h"
 

chrome/browser/sync/glue/synced_tab_delegate_android.cc

@@ -18,106 +18,36 @@ using content::NavigationEntry;
 
 namespace browser_sync {
 SyncedTabDelegateAndroid::SyncedTabDelegateAndroid(TabAndroid* tab_android)
-    : web_contents_(nullptr),
-      tab_android_(tab_android),
-      tab_contents_delegate_(nullptr) {
-}
+    : tab_android_(tab_android) {}
 
 SyncedTabDelegateAndroid::~SyncedTabDelegateAndroid() {}
 
-SessionID SyncedTabDelegateAndroid::GetWindowId() const {
-  return tab_contents_delegate_->GetWindowId();
-}
-
-SessionID SyncedTabDelegateAndroid::GetSessionId() const {
-  return tab_android_->session_id();
-}
-
-bool SyncedTabDelegateAndroid::IsBeingDestroyed() const {
-  return tab_contents_delegate_->IsBeingDestroyed();
-}
-
-SessionID SyncedTabDelegateAndroid::GetSourceTabID() const {
-  return tab_contents_delegate_->GetSourceTabID();
-}
-
-std::string SyncedTabDelegateAndroid::GetExtensionAppId() const {
-  return tab_contents_delegate_->GetExtensionAppId();
-}
-
-bool SyncedTabDelegateAndroid::IsInitialBlankNavigation() const {
-  return tab_contents_delegate_->IsInitialBlankNavigation();
-}
-
-int SyncedTabDelegateAndroid::GetCurrentEntryIndex() const {
-  return tab_contents_delegate_->GetCurrentEntryIndex();
-}
-
-int SyncedTabDelegateAndroid::GetEntryCount() const {
-  return tab_contents_delegate_->GetEntryCount();
-}
-
-GURL SyncedTabDelegateAndroid::GetVirtualURLAtIndex(int i) const {
-  return tab_contents_delegate_->GetVirtualURLAtIndex(i);
-}
-
-GURL SyncedTabDelegateAndroid::GetFaviconURLAtIndex(int i) const {
-  return tab_contents_delegate_->GetFaviconURLAtIndex(i);
-}
-
-ui::PageTransition SyncedTabDelegateAndroid::GetTransitionAtIndex(int i) const {
-  return tab_contents_delegate_->GetTransitionAtIndex(i);
+bool SyncedTabDelegateAndroid::IsPlaceholderTab() const {
+  return web_contents() == nullptr;
 }
 
-void SyncedTabDelegateAndroid::GetSerializedNavigationAtIndex(
-    int i,
-    sessions::SerializedNavigationEntry* serialized_entry) const {
-  tab_contents_delegate_->GetSerializedNavigationAtIndex(i, serialized_entry);
+int SyncedTabDelegateAndroid::GetSyncId() const {
+  return tab_android_->GetSyncId();
 }
 
-bool SyncedTabDelegateAndroid::IsPlaceholderTab() const {
-  return web_contents_ == nullptr;
+void SyncedTabDelegateAndroid::SetSyncId(int sync_id) {
+  tab_android_->SetSyncId(sync_id);
 }
 
 void SyncedTabDelegateAndroid::SetWebContents(
     content::WebContents* web_contents,
     content::WebContents* source_web_contents) {
-  web_contents_ = web_contents;
-  TabContentsSyncedTabDelegate::CreateForWebContents(web_contents_);
-  // Store the TabContentsSyncedTabDelegate object that was created.
-  tab_contents_delegate_ =
-      TabContentsSyncedTabDelegate::FromWebContents(web_contents_);
+  TabContentsSyncedTabDelegate::SetWebContents(web_contents);
+
   if (source_web_contents) {
     sync_sessions::SyncSessionsRouterTabHelper::FromWebContents(
         source_web_contents)
-        ->SetSourceTabIdForChild(web_contents_);
+        ->SetSourceTabIdForChild(web_contents);
   }
 }
 
 void SyncedTabDelegateAndroid::ResetWebContents() {
-  web_contents_ = nullptr;
-}
-
-bool SyncedTabDelegateAndroid::ProfileIsSupervised() const {
-  return tab_contents_delegate_->ProfileIsSupervised();
-}
-
-const std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>*
-SyncedTabDelegateAndroid::GetBlockedNavigations() const {
-  return tab_contents_delegate_->GetBlockedNavigations();
-}
-
-int SyncedTabDelegateAndroid::GetSyncId() const {
-  return tab_android_->GetSyncId();
-}
-
-void SyncedTabDelegateAndroid::SetSyncId(int sync_id) {
-  tab_android_->SetSyncId(sync_id);
-}
-
-bool SyncedTabDelegateAndroid::ShouldSync(
-    sync_sessions::SyncSessionsClient* sessions_client) {
-  return tab_contents_delegate_->ShouldSync(sessions_client);
+  TabContentsSyncedTabDelegate::SetWebContents(nullptr);
 }
 
 }  // namespace browser_sync

chrome/browser/sync/glue/synced_tab_delegate_android.h

@@ -9,63 +9,39 @@
 #include <string>
 #include <vector>
 
-#include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "components/sync_sessions/synced_tab_delegate.h"
-#include "content/public/browser/web_contents_user_data.h"
+#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
 
 namespace content {
 class WebContents;
 }
 
 class TabAndroid;
-class TabContentsSyncedTabDelegate;
 
 namespace browser_sync {
 // On Android a tab can exist even without web contents.
 
-// SyncedTabDelegateAndroid wraps TabContentsSyncedTabDelegate and provides
-// a method to set web contents later when tab is brought to memory.
-class SyncedTabDelegateAndroid : public sync_sessions::SyncedTabDelegate {
+// SyncedTabDelegateAndroid specializes TabContentsSyncedTabDelegate with
+// support for setting web contents on a late stage (for placeholder tabs),
+// when the tab is brought to memory.
+class SyncedTabDelegateAndroid : public TabContentsSyncedTabDelegate {
  public:
   explicit SyncedTabDelegateAndroid(TabAndroid* owning_tab_);
   ~SyncedTabDelegateAndroid() override;
 
   // SyncedTabDelegate:
-  SessionID GetWindowId() const override;
-  SessionID GetSessionId() const override;
-  bool IsBeingDestroyed() const override;
-  SessionID GetSourceTabID() const override;
-  std::string GetExtensionAppId() const override;
-  bool IsInitialBlankNavigation() const override;
-  int GetCurrentEntryIndex() const override;
-  int GetEntryCount() const override;
-  GURL GetVirtualURLAtIndex(int i) const override;
-  GURL GetFaviconURLAtIndex(int i) const override;
-  ui::PageTransition GetTransitionAtIndex(int i) const override;
-  void GetSerializedNavigationAtIndex(
-      int i,
-      sessions::SerializedNavigationEntry* serialized_entry) const override;
   bool IsPlaceholderTab() const override;
   int GetSyncId() const override;
   void SetSyncId(int sync_id) override;
-  bool ShouldSync(sync_sessions::SyncSessionsClient* sessions_client) override;
-  bool ProfileIsSupervised() const override;
-  const std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>*
-  GetBlockedNavigations() const override;
 
-  // Set the web contents for this tab. Also creates
-  // TabContentsSyncedTabDelegate for this tab and handles source tab id
-  // initialization.
-  virtual void SetWebContents(content::WebContents* web_contents,
-                              content::WebContents* source_web_contents);
+  // Set the web contents for this tab and handles source tab ID initialization.
+  void SetWebContents(content::WebContents* web_contents,
+                      content::WebContents* source_web_contents);
   // Set web contents to null.
-  virtual void ResetWebContents();
+  void ResetWebContents();
 
  private:
-  content::WebContents* web_contents_;
   TabAndroid* tab_android_;
-  TabContentsSyncedTabDelegate* tab_contents_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncedTabDelegateAndroid);
 };

chrome/browser/sync/sessions/sync_sessions_router_tab_helper_unittest.cc

@@ -9,7 +9,7 @@
 
 #include "chrome/browser/sync/sessions/sync_sessions_web_contents_router.h"
 #include "chrome/browser/sync/sessions/sync_sessions_web_contents_router_factory.h"
-#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/browser_synced_tab_delegate.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/browser/navigation_handle.h"
@@ -38,7 +38,7 @@ class FakeLocalSessionEventHandler : public LocalSessionEventHandler {
   }
 
  private:
-  bool was_notified_;
+  bool was_notified_ = false;
 };
 
 class SyncSessionsRouterTabHelperTest : public ChromeRenderViewHostTestHarness {
@@ -56,7 +56,7 @@ class SyncSessionsRouterTabHelperTest : public ChromeRenderViewHostTestHarness {
     SyncSessionsRouterTabHelper::CreateForWebContents(web_contents(), router_);
     router_->StartRoutingTo(handler());
 
-    TabContentsSyncedTabDelegate::CreateForWebContents(web_contents());
+    BrowserSyncedTabDelegate::CreateForWebContents(web_contents());
     NavigateAndCommit(GURL("about:blank"));
   }
 
@@ -72,7 +72,7 @@ TEST_F(SyncSessionsRouterTabHelperTest, SubframeNavigationsIgnored) {
   SyncSessionsRouterTabHelper* helper =
       SyncSessionsRouterTabHelper::FromWebContents(web_contents());
 
-  EXPECT_TRUE(handler()->was_notified_since_last_call());
+  ASSERT_TRUE(handler()->was_notified_since_last_call());
 
   content::RenderFrameHost* child_rfh =
       content::RenderFrameHostTester::For(main_rfh())->AppendChild("subframe");

chrome/browser/sync/sessions/sync_sessions_web_contents_router.cc

@@ -8,11 +8,10 @@
 #include "chrome/browser/profiles/profile.h"
 #if !defined(OS_ANDROID)
 #include "chrome/browser/sync/sessions/browser_list_router_helper.h"
+#include "chrome/browser/ui/sync/browser_synced_tab_delegate.h"
 #else
 #include "chrome/browser/android/tab_android.h"
 #endif  // !defined(OS_ANDROID)
-#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
-#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/sync_sessions/sync_sessions_client.h"
 #include "components/sync_sessions/synced_tab_delegate.h"
@@ -28,7 +27,7 @@ SyncedTabDelegate* GetSyncedTabDelegateFromWebContents(
   return tab ? tab->GetSyncedTabDelegate() : nullptr;
 #else
   SyncedTabDelegate* delegate =
-      TabContentsSyncedTabDelegate::FromWebContents(web_contents);
+      BrowserSyncedTabDelegate::FromWebContents(web_contents);
   return delegate;
 #endif
 }

chrome/browser/sync/sessions/sync_sessions_web_contents_router_unittest.cc

@@ -3,8 +3,10 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/sessions/sync_sessions_web_contents_router.h"
+
+#include "build/build_config.h"
 #include "chrome/browser/sync/sessions/sync_sessions_web_contents_router_factory.h"
-#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/browser_synced_tab_delegate.h"
 #include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/web_contents_tester.h"
@@ -47,6 +49,10 @@ class SyncSessionsWebContentsRouterTest : public testing::Test {
   std::unique_ptr<content::WebContents> test_contents_;
 };
 
+// Disabled on android due to complexity of creating a full TabAndroid object
+// for a unit test. The logic being tested here isn't directly affected by
+// platform-specific peculiarities.
+#if !defined(OS_ANDROID)
 TEST_F(SyncSessionsWebContentsRouterTest, FlareNotRun) {
   StartSyncFlareMock mock;
   router()->InjectStartSyncFlare(
@@ -56,7 +62,7 @@ TEST_F(SyncSessionsWebContentsRouterTest, FlareNotRun) {
   router()->NotifyTabModified(test_contents(), false);
   EXPECT_FALSE(mock.was_run());
 
-  TabContentsSyncedTabDelegate::CreateForWebContents(test_contents());
+  BrowserSyncedTabDelegate::CreateForWebContents(test_contents());
 
   // There's a delegate for the tab, but it's not a load completed event, so the
   // flare still shouldn't run.
@@ -66,16 +72,12 @@ TEST_F(SyncSessionsWebContentsRouterTest, FlareNotRun) {
 
 // Make sure we don't crash when there's not a flare.
 TEST_F(SyncSessionsWebContentsRouterTest, FlareNotSet) {
-  TabContentsSyncedTabDelegate::CreateForWebContents(test_contents());
+  BrowserSyncedTabDelegate::CreateForWebContents(test_contents());
   router()->NotifyTabModified(test_contents(), false);
 }
 
-// Disabled on android due to complexity of creating a full TabAndroid object
-// for a unit test. The logic being tested here isn't directly affected by
-// platform-specific peculiarities.
-#if !defined(OS_ANDROID)
 TEST_F(SyncSessionsWebContentsRouterTest, FlareRunsForLoadCompleted) {
-  TabContentsSyncedTabDelegate::CreateForWebContents(test_contents());
+  BrowserSyncedTabDelegate::CreateForWebContents(test_contents());
 
   StartSyncFlareMock mock;
   router()->InjectStartSyncFlare(

chrome/browser/ui/BUILD.gn

@@ -1474,6 +1474,8 @@ split_static_library("ui") {
       "startup/startup_tab_provider.cc",
       "startup/startup_tab_provider.h",
       "startup/startup_types.h",
+      "sync/browser_synced_tab_delegate.cc",
+      "sync/browser_synced_tab_delegate.h",
       "sync/browser_synced_window_delegate.cc",
       "sync/browser_synced_window_delegate.h",
       "sync/browser_synced_window_delegates_getter.cc",

chrome/browser/ui/sync/browser_synced_tab_delegate.cc

+// Copyright 2018 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/ui/sync/browser_synced_tab_delegate.h"
+
+#include "components/sync_sessions/tab_node_pool.h"
+
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(BrowserSyncedTabDelegate);
+
+BrowserSyncedTabDelegate::BrowserSyncedTabDelegate(
+    content::WebContents* web_contents)
+    : sync_id_(sync_sessions::TabNodePool::kInvalidTabNodeID) {
+  SetWebContents(web_contents);
+}
+
+BrowserSyncedTabDelegate::~BrowserSyncedTabDelegate() {}
+
+bool BrowserSyncedTabDelegate::IsPlaceholderTab() const {
+  return false;
+}
+
+int BrowserSyncedTabDelegate::GetSyncId() const {
+  return sync_id_;
+}
+
+void BrowserSyncedTabDelegate::SetSyncId(int sync_id) {
+  sync_id_ = sync_id;
+}

chrome/browser/ui/sync/browser_synced_tab_delegate.h

+// Copyright 2018 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_UI_SYNC_BROWSER_SYNCED_TAB_DELEGATE_H_
+#define CHROME_BROWSER_UI_SYNC_BROWSER_SYNCED_TAB_DELEGATE_H_
+
+#include "base/macros.h"
+#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
+#include "components/sessions/core/session_id.h"
+#include "content/public/browser/web_contents_user_data.h"
+
+namespace content {
+class WebContents;
+}
+
+// A BrowserSyncedTabDelegate is the Browser-based implementation of
+// SyncedTabDelegate.
+class BrowserSyncedTabDelegate
+    : public TabContentsSyncedTabDelegate,
+      public content::WebContentsUserData<BrowserSyncedTabDelegate> {
+ public:
+  ~BrowserSyncedTabDelegate() override;
+
+  // SyncedTabDelegate:
+  bool IsPlaceholderTab() const override;
+  int GetSyncId() const override;
+  void SetSyncId(int sync_id) override;
+
+ private:
+  explicit BrowserSyncedTabDelegate(content::WebContents* web_contents);
+  friend class content::WebContentsUserData<BrowserSyncedTabDelegate>;
+
+  int sync_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(BrowserSyncedTabDelegate);
+};
+
+#endif  // CHROME_BROWSER_UI_SYNC_BROWSER_SYNCED_TAB_DELEGATE_H_

chrome/browser/ui/sync/browser_synced_window_delegate.cc

@@ -8,7 +8,7 @@
 
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
-#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/browser_synced_tab_delegate.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 
 BrowserSyncedWindowDelegate::BrowserSyncedWindowDelegate(Browser* browser)
@@ -30,7 +30,7 @@ bool BrowserSyncedWindowDelegate::IsTabPinned(
 
 sync_sessions::SyncedTabDelegate* BrowserSyncedWindowDelegate::GetTabAt(
     int index) const {
-  return TabContentsSyncedTabDelegate::FromWebContents(
+  return BrowserSyncedTabDelegate::FromWebContents(
       browser_->tab_strip_model()->GetWebContentsAt(index));
 }
 

chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc

@@ -13,7 +13,6 @@
 #include "components/sync_sessions/sync_sessions_client.h"
 #include "components/sync_sessions/synced_window_delegate.h"
 #include "components/sync_sessions/synced_window_delegates_getter.h"
-#include "components/sync_sessions/tab_node_pool.h"
 #include "content/public/browser/favicon_status.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
@@ -31,8 +30,6 @@
 
 using content::NavigationEntry;
 
-DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabContentsSyncedTabDelegate);
-
 namespace {
 
 // Helper to access the correct NavigationEntry, accounting for pending entries.
@@ -47,10 +44,8 @@ NavigationEntry* GetPossiblyPendingEntryAtIndex(
 
 }  // namespace
 
-TabContentsSyncedTabDelegate::TabContentsSyncedTabDelegate(
-    content::WebContents* web_contents)
-    : web_contents_(web_contents),
-      sync_session_id_(sync_sessions::TabNodePool::kInvalidTabNodeID) {}
+TabContentsSyncedTabDelegate::TabContentsSyncedTabDelegate()
+    : web_contents_(nullptr) {}
 
 TabContentsSyncedTabDelegate::~TabContentsSyncedTabDelegate() {}
 
@@ -142,18 +137,6 @@ TabContentsSyncedTabDelegate::GetBlockedNavigations() const {
 #endif
 }
 
-bool TabContentsSyncedTabDelegate::IsPlaceholderTab() const {
-  return false;
-}
-
-int TabContentsSyncedTabDelegate::GetSyncId() const {
-  return sync_session_id_;
-}
-
-void TabContentsSyncedTabDelegate::SetSyncId(int sync_id) {
-  sync_session_id_ = sync_id;
-}
-
 bool TabContentsSyncedTabDelegate::ShouldSync(
     sync_sessions::SyncSessionsClient* sessions_client) {
   if (sessions_client->GetSyncedWindowDelegatesGetter()->FindById(
@@ -185,3 +168,16 @@ SessionID TabContentsSyncedTabDelegate::GetSourceTabID() const {
           web_contents_);
   return helper->source_tab_id();
 }
+
+const content::WebContents* TabContentsSyncedTabDelegate::web_contents() const {
+  return web_contents_;
+}
+
+content::WebContents* TabContentsSyncedTabDelegate::web_contents() {
+  return web_contents_;
+}
+
+void TabContentsSyncedTabDelegate::SetWebContents(
+    content::WebContents* web_contents) {
+  web_contents_ = web_contents;
+}

chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h

@@ -9,20 +9,19 @@
 #include <string>
 #include <vector>
 
-#include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "components/sessions/core/session_id.h"
 #include "components/sync_sessions/synced_tab_delegate.h"
-#include "content/public/browser/web_contents_user_data.h"
 
 namespace content {
 class WebContents;
 }
 
-class TabContentsSyncedTabDelegate
-    : public sync_sessions::SyncedTabDelegate,
-      public content::WebContentsUserData<TabContentsSyncedTabDelegate> {
+// Partial implementation of SyncedTabDelegate for the cases where the tab has
+// (either initially or late) a WebContents.
+class TabContentsSyncedTabDelegate : public sync_sessions::SyncedTabDelegate {
  public:
+  TabContentsSyncedTabDelegate();
   ~TabContentsSyncedTabDelegate() override;
 
   // SyncedTabDelegate:
@@ -42,18 +41,16 @@ class TabContentsSyncedTabDelegate
   bool ProfileIsSupervised() const override;
   const std::vector<std::unique_ptr<const sessions::SerializedNavigationEntry>>*
   GetBlockedNavigations() const override;
-  bool IsPlaceholderTab() const override;
-  int GetSyncId() const override;
-  void SetSyncId(int sync_id) override;
   bool ShouldSync(sync_sessions::SyncSessionsClient* sessions_client) override;
   SessionID GetSourceTabID() const override;
 
- private:
-  explicit TabContentsSyncedTabDelegate(content::WebContents* web_contents);
-  friend class content::WebContentsUserData<TabContentsSyncedTabDelegate>;
+ protected:
+  const content::WebContents* web_contents() const;
+  content::WebContents* web_contents();
+  void SetWebContents(content::WebContents* web_contents);
 
+ private:
   content::WebContents* web_contents_;
-  int sync_session_id_;
 
   DISALLOW_COPY_AND_ASSIGN(TabContentsSyncedTabDelegate);
 };

chrome/browser/ui/sync/tab_contents_synced_tab_delegate_unittest.cc

@@ -5,10 +5,24 @@
 #include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
 
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "content/public/browser/web_contents.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
 
+class TestSyncedTabDelegate : public TabContentsSyncedTabDelegate {
+ public:
+  explicit TestSyncedTabDelegate(content::WebContents* web_contents) {
+    SetWebContents(web_contents);
+  }
+
+  ~TestSyncedTabDelegate() override {}
+
+  bool IsPlaceholderTab() const override { return false; }
+  int GetSyncId() const override { return -1; }
+  void SetSyncId(int sync_id) override {}
+};
+
 class TabContentsSyncedTabDelegateTest
     : public ChromeRenderViewHostTestHarness {
  public:
@@ -19,28 +33,23 @@ class TabContentsSyncedTabDelegateTest
     content::RenderViewHostTestHarness::SetUp();
 
     NavigateAndCommit(GURL("about:blank"));
-    TabContentsSyncedTabDelegate::CreateForWebContents(web_contents());
   }
 };
 
 TEST_F(TabContentsSyncedTabDelegateTest, InvalidEntryIndexReturnsDefault) {
   std::unique_ptr<content::WebContents> web_contents(CreateTestWebContents());
-
-  TabContentsSyncedTabDelegate::CreateForWebContents(web_contents.get());
-
-  TabContentsSyncedTabDelegate* delegate =
-      TabContentsSyncedTabDelegate::FromWebContents(web_contents.get());
+  TestSyncedTabDelegate delegate(web_contents.get());
 
   // -1 and 2 are invalid indices because there's only one navigation
   // recorded(the initial one to "about:blank")
-  EXPECT_EQ(delegate->GetFaviconURLAtIndex(-1), GURL());
-  EXPECT_EQ(delegate->GetFaviconURLAtIndex(2), GURL());
+  EXPECT_EQ(delegate.GetFaviconURLAtIndex(-1), GURL());
+  EXPECT_EQ(delegate.GetFaviconURLAtIndex(2), GURL());
 
   EXPECT_TRUE(
-      PageTransitionCoreTypeIs(delegate->GetTransitionAtIndex(-1),
+      PageTransitionCoreTypeIs(delegate.GetTransitionAtIndex(-1),
                                ui::PageTransition::PAGE_TRANSITION_LINK));
   EXPECT_TRUE(
-      PageTransitionCoreTypeIs(delegate->GetTransitionAtIndex(2),
+      PageTransitionCoreTypeIs(delegate.GetTransitionAtIndex(2),
                                ui::PageTransition::PAGE_TRANSITION_LINK));
 }
 

chrome/browser/ui/tab_helpers.cc

@@ -108,7 +108,7 @@
 #include "chrome/browser/ui/hung_plugin_tab_helper.h"
 #include "chrome/browser/ui/sad_tab_helper.h"
 #include "chrome/browser/ui/search/search_tab_helper.h"
-#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/browser_synced_tab_delegate.h"
 #include "components/pdf/browser/pdf_web_contents_helper.h"
 #include "components/web_modal/web_contents_modal_dialog_manager.h"
 #include "components/zoom/zoom_controller.h"
@@ -276,6 +276,7 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
   ViewAndroidHelper::CreateForWebContents(web_contents);
 #else
   BookmarkTabHelper::CreateForWebContents(web_contents);
+  BrowserSyncedTabDelegate::CreateForWebContents(web_contents);
   extensions::ChromeExtensionWebContentsObserver::CreateForWebContents(
       web_contents);
   extensions::WebNavigationTabObserver::CreateForWebContents(web_contents);
@@ -292,7 +293,6 @@ void TabHelpers::AttachTabHelpers(WebContents* web_contents) {
   safe_browsing::SafeBrowsingNavigationObserver::MaybeCreateForWebContents(
       web_contents);
   SearchTabHelper::CreateForWebContents(web_contents);
-  TabContentsSyncedTabDelegate::CreateForWebContents(web_contents);
   TabDialogs::CreateForWebContents(web_contents);
   ThumbnailTabHelper::CreateForWebContents(web_contents);
   web_modal::WebContentsModalDialogManager::CreateForWebContents(web_contents);