New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 599548 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Number of Open tabs in Chrome sync dashboard wrong

Project Member Reported by jainabhi...@chromium.org, Mar 31 2016

Issue description

Version: M50
OS: CROS

What steps will reproduce the problem?
(1) Open Chrome
(2) Navigate to https://www.google.com/settings/chrome/sync
(3) Scroll to find Open tabs

What is the expected output?
I should see exact number of open tabs on all devices

What do you see instead?
I see a huge number (400+) of open tabs across my devices. I dont open more than 15 - 20 tabs at a time on a really busy day.

 

Comment 1 by skym@google.com, Mar 31 2016

Poking around, it looks mainly to be because we have old data that's not partially deleted. We "age out" sessions that haven't had any changes in the last 14 days. We try to issue deletes for these, but it looks like it's possible for this logic to miss old data. https://code.google.com/p/chromium/codesearch#chromium/src/components/sync_sessions/sessions_sync_manager.cc&sq=package:chromium&type=cs&l=1031

Looking at chrome://sync-internals, you can look at the session data, it will match the number that the dashboard claims. It looks like a couple issues are compounding here. Old windows that no longer have any valid tabs are never deleted. Tabs sometimes don't get deleted when they're removed from their parent window, and these never get cleaned up either.

Trying to dig into how exactly the session tracker works, things get a bunch harrier. Some of my data is considered 'orphaned' and sits there. Some of it gets deleted initially in DeleteOldSessionTabIfNecessary. And some of it I lose track of and I'm not exactly sure where it goes, but it's definitely not being returned as part of LookupAllForeignSessions , which is what the age out logic uses.

The impact this issue is causing
1) Dashboard claims a large number, confusing.
2) New clients have to download these old pieces of data.
3) We hold some of these old pieces of data in memory, all of them on disk, and all of them over the wire on new client syncs.

Comment 2 by s...@chromium.org, Apr 6 2016

Owner: s...@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by s...@chromium.org, Apr 7 2016

Did some more poking around, I think I understand all the different categories of useless data

* Foreign windows/headers without any syncable tabs
* Orphaned tabs (some of these are cleared during session_tracker_.CleanupSession, some of them are not)
* Duplicate tabs (different sync id, same tab_id and session_tag)

The first two should be easy to fix, we can simply be more inclusive in our deletion of foreign items and less aggressive with cleanup. Not sure if it's worth it to directly address duplicate foreign tabs (by deleting the older entry), since they'll be orphaned after 14 days anyways.

Comment 4 by s...@chromium.org, Apr 12 2016

Okay, I've posted a CL that should address what can be addressed here. On my personal account my changes took the number of tabs from ~300 to ~170 (I actually have ~40 tabs that can be opened). This should be a memory/disk space win for all users syncing sessions.

However, these session counts will never able to truly converge. You may have anywhere between 0 and 100 orphaned pieces of tab data, per device, that sits around and do nothing. What's going on is that, because Sessions is such a highly modified data type, we reuse identifiers. Instead of deleting some tabs we will simply un-parent it, and then later we will re-use the identifier with completely new data. This is working as intended, and I don't think we want to change this behavior.
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 14 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f

commit 199c7fdea8b1652b9ff0d17cdd2989476b63cf5f
Author: skym <skym@chromium.org>
Date: Thu Apr 14 19:26:42 2016

[Sync] Moved tab_node_id tracking to session object and improved foreign session
garbage collection.

Orphaned tabs and empty header nodes were previously completely missed by
garbage collection, leading to a bloat of sync data. Additionally, orphan tabs
that were added after the header during merge were never cleaned up by the
tracker, using memory. These issues should all be resolved, but in order to
correctly perform garbage collection of cleaned up session data, the location
of tab_node_ids were moved from the SessionTabWrapper to the SyncedSession so
that they can be tracked through cleanup calls. Should expect a minor memory
improvement for users syncing session data.

BUG= 599548 

Review URL: https://codereview.chromium.org/1877083002

Cr-Commit-Position: refs/heads/master@{#387391}

[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/chrome/browser/sync/glue/session_sync_test_helper.cc
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/chrome/browser/sync/glue/session_sync_test_helper.h
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/chrome/browser/sync/sessions/sessions_sync_manager_unittest.cc
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/components/sync_sessions/sessions_sync_manager.cc
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/components/sync_sessions/sessions_sync_manager.h
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/components/sync_sessions/synced_session.h
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/components/sync_sessions/synced_session_tracker.cc
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/components/sync_sessions/synced_session_tracker.h
[modify] https://crrev.com/199c7fdea8b1652b9ff0d17cdd2989476b63cf5f/components/sync_sessions/synced_session_tracker_unittest.cc

Comment 6 by s...@chromium.org, Apr 14 2016

Status: Fixed (was: Assigned)

Sign in to add a comment