Additional tab synchronization nodes created on restart
Reported by
akalu...@yandex-team.ru,
May 29 2018
|
||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3359.181 YaBrowser/18.6.1.209 (beta) Yowser/2.5 Safari/537.36 Steps to reproduce the problem: Steps to reproduce: - install browser, enable "Continue where you left off" option (reopen all tabs on restart) and synchronization of tabs - open 3 new tabs - go to sync-internals and verify you have 4 nodes in "Sessions" directory (one for header and 3 for tabs) - restart browser, wait for synchronization to complete - go to sync-internals and verify current sessions state Expected behavior: - you still have 4 synchronization nodes in "Sessions" directory Actual behavior: - you have 7 synchronization nodes - 1 for header, 3 for tabs, and 3 free nodes - if you restart browser once again you will get another 3 additional nodes. What is the expected behavior? you still have 4 synchronization nodes in "Sessions" directory What went wrong? you have 7 synchronization nodes - 1 for header, 3 for tabs, and 3 free nodes. Did this work before? No Chrome version: 66.0.3359.181 Channel: stable OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Shockwave Flash 27.0 r0 Observation: after several restarts additional nodes are not created any more, but I believe this happens due to some issues in current synchronization state tracking.
,
Jul 2
triage ping, thanks
,
Aug 6
sync-triage ping: any updates?
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c8e98722a8f88e4669cff3c406813fee6688407c commit c8e98722a8f88e4669cff3c406813fee6688407c Author: Mikel Astiz <mastiz@chromium.org> Date: Wed Sep 05 11:08:29 2018 Fix tab nodes not marked as free during restore When the local session is restored from persistence, if there are orphan tabs, they should be marked as free, to prevent newly created tabs from creating new tab nodes, instead of recycling available ones. The restore flow exercises SyncedSessionTracker::CleanupSession(), which prior to this patch wasn't smart enough for the local session. Bug: 875671 , 847325 Change-Id: I91f9f13ad4aedbbd85bfcc73c606096781b80527 Reviewed-on: https://chromium-review.googlesource.com/1204134 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Commit-Position: refs/heads/master@{#588827} [modify] https://crrev.com/c8e98722a8f88e4669cff3c406813fee6688407c/components/sync_sessions/session_sync_bridge_unittest.cc [modify] https://crrev.com/c8e98722a8f88e4669cff3c406813fee6688407c/components/sync_sessions/synced_session_tracker.cc [modify] https://crrev.com/c8e98722a8f88e4669cff3c406813fee6688407c/components/sync_sessions/synced_session_tracker.h [modify] https://crrev.com/c8e98722a8f88e4669cff3c406813fee6688407c/components/sync_sessions/test_synced_window_delegates_getter.cc [modify] https://crrev.com/c8e98722a8f88e4669cff3c406813fee6688407c/components/sync_sessions/test_synced_window_delegates_getter.h
,
Sep 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80000c1f89dc2b0b3a0997c1b5321a29402aa977 commit 80000c1f89dc2b0b3a0997c1b5321a29402aa977 Author: Mikel Astiz <mastiz@chromium.org> Date: Sat Sep 08 06:35:23 2018 Fix tab nodes not marked as free during restore When the local session is restored from persistence, if there are orphan tabs, they should be marked as free, to prevent newly created tabs from creating new tab nodes, instead of recycling available ones. The restore flow exercises SyncedSessionTracker::CleanupSession(), which prior to this patch wasn't smart enough for the local session. Bug: 875671 , 847325 Change-Id: I91f9f13ad4aedbbd85bfcc73c606096781b80527 Reviewed-on: https://chromium-review.googlesource.com/1204134 Commit-Queue: Mikel Astiz <mastiz@chromium.org> Reviewed-by: Marc Treib <treib@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#588827}(cherry picked from commit c8e98722a8f88e4669cff3c406813fee6688407c) Reviewed-on: https://chromium-review.googlesource.com/1215022 Reviewed-by: Mikel Astiz <mastiz@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#185} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/80000c1f89dc2b0b3a0997c1b5321a29402aa977/components/sync_sessions/session_sync_bridge_unittest.cc [modify] https://crrev.com/80000c1f89dc2b0b3a0997c1b5321a29402aa977/components/sync_sessions/synced_session_tracker.cc [modify] https://crrev.com/80000c1f89dc2b0b3a0997c1b5321a29402aa977/components/sync_sessions/synced_session_tracker.h [modify] https://crrev.com/80000c1f89dc2b0b3a0997c1b5321a29402aa977/components/sync_sessions/test_synced_window_delegates_getter.cc [modify] https://crrev.com/80000c1f89dc2b0b3a0997c1b5321a29402aa977/components/sync_sessions/test_synced_window_delegates_getter.h
,
Sep 26
mastiz@, is this fixed?
,
Sep 27
Thanks for the reminder. I'll need to take one more look here: the patch above fixed the most important issue, I believe, which is about the number of tab nodes growing indefinitely. The integration tests proposed in the patch linked in #c1 wouldn't pass, which makes it obvious that there is room for improvement.
,
Dec 4
Although there is still room for improvement, and considering it's unlikely that we improve things further, I'm closing this bug. |
||||
►
Sign in to add a comment |
||||
Comment 1 by mastiz@chromium.org
, May 29 2018Labels: Sync-Triaged
Status: Available (was: Unconfirmed)