Revert "Avoid a proxy tab helper on Android for sync delegation"

This reverts commit 42c62276.

Reason for revert: suspect for browser crashes in canary as per
crbug.com/853049.

Original change's description:
> 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}

TBR=avi@chromium.org,treib@chromium.org,mastiz@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 851905,853049
Change-Id: I71a78fa8fd50b551c5ae50cb64f1fe5e033383e5
Reviewed-on: https://chromium-review.googlesource.com/1102537Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567662}

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

@@ -7,7 +7,6 @@
 #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,7 +15,6 @@
 #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,36 +18,106 @@ using content::NavigationEntry;
 
 namespace browser_sync {
 SyncedTabDelegateAndroid::SyncedTabDelegateAndroid(TabAndroid* tab_android)
-    : tab_android_(tab_android) {}
+    : web_contents_(nullptr),
+      tab_android_(tab_android),
+      tab_contents_delegate_(nullptr) {
+}
 
 SyncedTabDelegateAndroid::~SyncedTabDelegateAndroid() {}
 
-bool SyncedTabDelegateAndroid::IsPlaceholderTab() const {
-  return web_contents() == nullptr;
+SessionID SyncedTabDelegateAndroid::GetWindowId() const {
+  return tab_contents_delegate_->GetWindowId();
 }
 
-int SyncedTabDelegateAndroid::GetSyncId() const {
-  return tab_android_->GetSyncId();
+SessionID SyncedTabDelegateAndroid::GetSessionId() const {
+  return tab_android_->session_id();
 }
 
-void SyncedTabDelegateAndroid::SetSyncId(int sync_id) {
-  tab_android_->SetSyncId(sync_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);
+}
+
+void SyncedTabDelegateAndroid::GetSerializedNavigationAtIndex(
+    int i,
+    sessions::SerializedNavigationEntry* serialized_entry) const {
+  tab_contents_delegate_->GetSerializedNavigationAtIndex(i, serialized_entry);
+}
+
+bool SyncedTabDelegateAndroid::IsPlaceholderTab() const {
+  return web_contents_ == nullptr;
 }
 
 void SyncedTabDelegateAndroid::SetWebContents(
     content::WebContents* web_contents,
     content::WebContents* source_web_contents) {
-  TabContentsSyncedTabDelegate::SetWebContents(web_contents);
-
+  web_contents_ = web_contents;
+  TabContentsSyncedTabDelegate::CreateForWebContents(web_contents_);
+  // Store the TabContentsSyncedTabDelegate object that was created.
+  tab_contents_delegate_ =
+      TabContentsSyncedTabDelegate::FromWebContents(web_contents_);
   if (source_web_contents) {
     sync_sessions::SyncSessionsRouterTabHelper::FromWebContents(
         source_web_contents)
-        ->SetSourceTabIdForChild(web_contents);
+        ->SetSourceTabIdForChild(web_contents_);
   }
 }
 
 void SyncedTabDelegateAndroid::ResetWebContents() {
-  TabContentsSyncedTabDelegate::SetWebContents(nullptr);
+  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);
 }
 
 }  // namespace browser_sync

chrome/browser/sync/glue/synced_tab_delegate_android.h

@@ -9,39 +9,63 @@
 #include <string>
 #include <vector>
 
+#include "base/compiler_specific.h"
 #include "base/macros.h"
-#include "chrome/browser/ui/sync/tab_contents_synced_tab_delegate.h"
+#include "components/sync_sessions/synced_tab_delegate.h"
+#include "content/public/browser/web_contents_user_data.h"
 
 namespace content {
 class WebContents;
 }
 
 class TabAndroid;
+class TabContentsSyncedTabDelegate;
 
 namespace browser_sync {
 // On Android a tab can exist even without web contents.
 
-// 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 {
+// SyncedTabDelegateAndroid wraps TabContentsSyncedTabDelegate and provides
+// a method to set web contents later when tab is brought to memory.
+class SyncedTabDelegateAndroid : public sync_sessions::SyncedTabDelegate {
  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 and handles source tab ID initialization.
-  void SetWebContents(content::WebContents* web_contents,
-                      content::WebContents* source_web_contents);
+  // 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 web contents to null.
-  void ResetWebContents();
+  virtual 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/browser_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/tab_contents_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_ = false;
+  bool was_notified_;
 };
 
 class SyncSessionsRouterTabHelperTest : public ChromeRenderViewHostTestHarness {
@@ -56,7 +56,7 @@ class SyncSessionsRouterTabHelperTest : public ChromeRenderViewHostTestHarness {
     SyncSessionsRouterTabHelper::CreateForWebContents(web_contents(), router_);
     router_->StartRoutingTo(handler());
 
-    BrowserSyncedTabDelegate::CreateForWebContents(web_contents());
+    TabContentsSyncedTabDelegate::CreateForWebContents(web_contents());
     NavigateAndCommit(GURL("about:blank"));
   }
 
@@ -72,7 +72,7 @@ TEST_F(SyncSessionsRouterTabHelperTest, SubframeNavigationsIgnored) {
   SyncSessionsRouterTabHelper* helper =
       SyncSessionsRouterTabHelper::FromWebContents(web_contents());
 
-  ASSERT_TRUE(handler()->was_notified_since_last_call());
+  EXPECT_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,10 +8,11 @@
 #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"
@@ -27,7 +28,7 @@ SyncedTabDelegate* GetSyncedTabDelegateFromWebContents(
   return tab ? tab->GetSyncedTabDelegate() : nullptr;
 #else
   SyncedTabDelegate* delegate =
-      BrowserSyncedTabDelegate::FromWebContents(web_contents);
+      TabContentsSyncedTabDelegate::FromWebContents(web_contents);
   return delegate;
 #endif
 }

chrome/browser/sync/sessions/sync_sessions_web_contents_router_unittest.cc

@@ -3,10 +3,8 @@
 // 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/browser_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/tab_contents_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"
@@ -49,10 +47,6 @@ 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(
@@ -62,7 +56,7 @@ TEST_F(SyncSessionsWebContentsRouterTest, FlareNotRun) {
   router()->NotifyTabModified(test_contents(), false);
   EXPECT_FALSE(mock.was_run());
 
-  BrowserSyncedTabDelegate::CreateForWebContents(test_contents());
+  TabContentsSyncedTabDelegate::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.
@@ -72,12 +66,16 @@ TEST_F(SyncSessionsWebContentsRouterTest, FlareNotRun) {
 
 // Make sure we don't crash when there's not a flare.
 TEST_F(SyncSessionsWebContentsRouterTest, FlareNotSet) {
-  BrowserSyncedTabDelegate::CreateForWebContents(test_contents());
+  TabContentsSyncedTabDelegate::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) {
-  BrowserSyncedTabDelegate::CreateForWebContents(test_contents());
+  TabContentsSyncedTabDelegate::CreateForWebContents(test_contents());
 
   StartSyncFlareMock mock;
   router()->InjectStartSyncFlare(

chrome/browser/ui/BUILD.gn

@@ -1474,8 +1474,6 @@ 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/browser_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/tab_contents_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 BrowserSyncedTabDelegate::FromWebContents(
+  return TabContentsSyncedTabDelegate::FromWebContents(
       browser_->tab_strip_model()->GetWebContentsAt(index));
 }
 

chrome/browser/ui/sync/tab_contents_synced_tab_delegate.cc

@@ -13,6 +13,7 @@
 #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"
@@ -30,6 +31,8 @@
 
 using content::NavigationEntry;
 
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabContentsSyncedTabDelegate);
+
 namespace {
 
 // Helper to access the correct NavigationEntry, accounting for pending entries.
@@ -44,8 +47,10 @@ NavigationEntry* GetPossiblyPendingEntryAtIndex(
 
 }  // namespace
 
-TabContentsSyncedTabDelegate::TabContentsSyncedTabDelegate()
-    : web_contents_(nullptr) {}
+TabContentsSyncedTabDelegate::TabContentsSyncedTabDelegate(
+    content::WebContents* web_contents)
+    : web_contents_(web_contents),
+      sync_session_id_(sync_sessions::TabNodePool::kInvalidTabNodeID) {}
 
 TabContentsSyncedTabDelegate::~TabContentsSyncedTabDelegate() {}
 
@@ -137,6 +142,18 @@ 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(
@@ -168,16 +185,3 @@ 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,19 +9,20 @@
 #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;
 }
 
-// Partial implementation of SyncedTabDelegate for the cases where the tab has
-// (either initially or late) a WebContents.
-class TabContentsSyncedTabDelegate : public sync_sessions::SyncedTabDelegate {
+class TabContentsSyncedTabDelegate
+    : public sync_sessions::SyncedTabDelegate,
+      public content::WebContentsUserData<TabContentsSyncedTabDelegate> {
  public:
-  TabContentsSyncedTabDelegate();
   ~TabContentsSyncedTabDelegate() override;
 
   // SyncedTabDelegate:
@@ -41,16 +42,18 @@ class TabContentsSyncedTabDelegate : public sync_sessions::SyncedTabDelegate {
   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;
 
- protected:
-  const content::WebContents* web_contents() const;
-  content::WebContents* web_contents();
-  void SetWebContents(content::WebContents* web_contents);
-
  private:
+  explicit TabContentsSyncedTabDelegate(content::WebContents* web_contents);
+  friend class content::WebContentsUserData<TabContentsSyncedTabDelegate>;
+
   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,24 +5,10 @@
 #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:
@@ -33,23 +19,28 @@ 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());
-  TestSyncedTabDelegate delegate(web_contents.get());
+
+  TabContentsSyncedTabDelegate::CreateForWebContents(web_contents.get());
+
+  TabContentsSyncedTabDelegate* delegate =
+      TabContentsSyncedTabDelegate::FromWebContents(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/browser_synced_tab_delegate.h"
+#include "chrome/browser/ui/sync/tab_contents_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,7 +276,6 @@ 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);
@@ -293,6 +292,7 @@ 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);