SyncSessionsWebContentsRouter creates TabContentsSyncedTabDelegates on Android |
||||||
Issue descriptionThe SyncSessionsWebContentsRouter is directly creating a TabContentsSyncedTabDelegate object from the WebContents. This is incorrect on Android, which needs to use SyncedTabDelegateAndroid objects that have a link to the TabAndroid object. In particular, this breaks the ability to Set/Get the sync id of the tab, which is necessary for dealing with placeholder tabs on Android.
,
Apr 17 2017
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/66da2b8252ada18ba7e5644eeee0ee089977d3be commit 66da2b8252ada18ba7e5644eeee0ee089977d3be Author: pnoland <pnoland@chromium.org> Date: Mon Apr 17 18:57:32 2017 [sync] Instantiate the platform appropriate SyncedTabDelegate SyncSessionsWebContentsRouter accesses an instance of the TabContentsSyncedTabDelegate directly on Android when it should use SyncedTabDelegateAndroid instead. This change fixes that. R=zea@chromium.org BUG= 711847 Review-Url: https://codereview.chromium.org/2823843003 Cr-Commit-Position: refs/heads/master@{#464963} [modify] https://crrev.com/66da2b8252ada18ba7e5644eeee0ee089977d3be/chrome/browser/sync/sessions/sync_sessions_web_contents_router.cc
,
Apr 17 2017
,
Apr 17 2017
Could we merge this fix into 59?
,
Apr 17 2017
,
Apr 17 2017
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c830170807ad321ff02c1e6e023fe7a0b1e4c43d commit c830170807ad321ff02c1e6e023fe7a0b1e4c43d Author: Sky Malice <skym@chromium.org> Date: Mon Apr 17 23:38:32 2017 [sync] Instantiate the platform appropriate SyncedTabDelegate SyncSessionsWebContentsRouter accesses an instance of the TabContentsSyncedTabDelegate directly on Android when it should use SyncedTabDelegateAndroid instead. This change fixes that. R=zea@chromium.org BUG= 711847 Review-Url: https://codereview.chromium.org/2823843003 Cr-Commit-Position: refs/heads/master@{#464963} (cherry picked from commit 66da2b8252ada18ba7e5644eeee0ee089977d3be) Review-Url: https://codereview.chromium.org/2824063002 . Cr-Commit-Position: refs/branch-heads/3071@{#27} Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641} [modify] https://crrev.com/c830170807ad321ff02c1e6e023fe7a0b1e4c43d/chrome/browser/sync/sessions/sync_sessions_web_contents_router.cc
,
Apr 20 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c2a141ce2ea4ea1a2826861798370bb91866f620 commit c2a141ce2ea4ea1a2826861798370bb91866f620 Author: pnoland <pnoland@chromium.org> Date: Thu Apr 20 22:21:41 2017 [sync] Add android test that ensures tabs get a valid sync id Add an OpenTabsTest that checks the sync id for a new tab, making sure it's not invalid. This test fails with my previous change reverted. R=zea@chromium.org BUG= 711847 Review-Url: https://codereview.chromium.org/2833763003 Cr-Commit-Position: refs/heads/master@{#466161} [modify] https://crrev.com/c2a141ce2ea4ea1a2826861798370bb91866f620/chrome/android/sync_shell/javatests/src/org/chromium/chrome/browser/sync/OpenTabsTest.java |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by zea@chromium.org
, Apr 15 2017