Reland "Avoid a proxy tab helper on Android for sync delegation"
This is a reland of 42c62276f6c54c5fd6e8142f4476568f03477243
The underlying issue in SyncedTabDelegateAndroid::GetSessionId()
has been reverted, which introduced an accidental behavioral
difference in the original version, causing crashes on Android
for placeholder tabs (i.e. tabs without WebContents).
[email protected],[email protected]
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 <[email protected]>
> Reviewed-by: Avi Drissman <[email protected]>
> Reviewed-by: Marc Treib <[email protected]>
> Cr-Commit-Position: refs/heads/master@{#567213}
Bug: 851905
Change-Id: I9e060ecf1272fec1ceebd4353fd390b95e64b669
Reviewed-on: https://chromium-review.googlesource.com/1103717
Reviewed-by: Mikel Astiz <[email protected]>
Commit-Queue: Mikel Astiz <[email protected]>
Cr-Commit-Position: refs/heads/master@{#567966}diff --git a/chrome/browser/ui/sync/browser_synced_tab_delegate.cc b/chrome/browser/ui/sync/browser_synced_tab_delegate.cc
new file mode 100644
index 0000000..d255510
--- /dev/null
+++ b/chrome/browser/ui/sync/browser_synced_tab_delegate.cc
@@ -0,0 +1,38 @@
+// 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 "chrome/browser/sessions/session_tab_helper.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() {}
+
+SessionID BrowserSyncedTabDelegate::GetWindowId() const {
+ return SessionTabHelper::FromWebContents(web_contents())->window_id();
+}
+
+SessionID BrowserSyncedTabDelegate::GetSessionId() const {
+ return SessionTabHelper::FromWebContents(web_contents())->session_id();
+}
+
+bool BrowserSyncedTabDelegate::IsPlaceholderTab() const {
+ return false;
+}
+
+int BrowserSyncedTabDelegate::GetSyncId() const {
+ return sync_id_;
+}
+
+void BrowserSyncedTabDelegate::SetSyncId(int sync_id) {
+ sync_id_ = sync_id;
+}
