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

Issue 707978 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----

Blocked on:
issue 736994



Sign in to add a comment

Save task id and ancestor navigations via session sync to backend data storage

Project Member Reported by shenchao@chromium.org, Apr 3 2017

Issue description

Currently, there is no explicit relationship between navigations in backend data showing one navigation is got by clicking a link from another. Such relationship helps us define tasks as a tree structure of navigations, which is a potentially better way to present navigation history than a sequential list by time.

To do this we can add a int64 field task_id and a repeated int64 field ancestor_navigation_task_id in TabNavigation proto, and populate them in SessionsSyncManager, before sending SessionSpecifics to backend.

Reason that we add all ancestors instead of just parent is that it makes possible to get the whole path to root by one query of data storage.

We considered to use global_id, but it is changed when doing forward_back, and in our context we think that they are the same navigation.




 

Comment 1 by zea@chromium.org, Apr 3 2017

Is this the same as the task id?
Yes. Since we are using existing global_id of navigation, it seems not necessary to introduce task id. I'll update the cl, where I didn't find the place to put task_id. 
Description: Show this description
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2017

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

commit 965fab94d2a8faef6f9ed4c6feaf99ef18e81049
Author: shenchao <shenchao@chromium.org>
Date: Thu Apr 20 23:29:33 2017

Add taskids for navigation, created in session sync.

This is the first step to introduce task in Chrome, which links and
groups navigations. Each navigation has a task id and a list of its
ancestor's task ids. A navigation's task id is TabNavigation's global
id when the navigation is first visited and then reused by going
back/forward visits. The task ids are created and sent to backend in
session sync.

And for now we only track navigation relationship in a tab.

BUG= 707978 

Review-Url: https://codereview.chromium.org/2776633003
Cr-Commit-Position: refs/heads/master@{#466188}

[modify] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync/protocol/session_specifics.proto
[modify] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/BUILD.gn
[modify] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/sessions_sync_manager.cc
[modify] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/sessions_sync_manager.h
[modify] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/sessions_sync_manager_unittest.cc
[add] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/task_tracker.cc
[add] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/task_tracker.h
[add] https://crrev.com/965fab94d2a8faef6f9ed4c6feaf99ef18e81049/components/sync_sessions/task_tracker_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 24 2017

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

commit 4c16aaa21c6f8a4cc6b392a9474fda426f9f36a1
Author: zea <zea@chromium.org>
Date: Mon Apr 24 17:45:58 2017

[Sync] Add task ids to proto visitor for tab sync

Proto visitors allow seeing the sync data in chrome://sync-internals

BUG= 707978 

Review-Url: https://codereview.chromium.org/2836573003
Cr-Commit-Position: refs/heads/master@{#466676}

[modify] https://crrev.com/4c16aaa21c6f8a4cc6b392a9474fda426f9f36a1/components/sync/protocol/proto_visitors.h

Project Member

Comment 6 by bugdroid1@chromium.org, May 18 2017

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

commit 854b8bcfc787413293c86eddd979a3fd1936fb1a
Author: shenchao <shenchao@chromium.org>
Date: Thu May 18 04:42:58 2017

Track task ids for navigations cross multiple tabs.

This is a following step to generate task ids for navigations. Now we
can track navigation relationship cross multiple tabs. If navigation B
in tab2 is clicked from navigation A in tab1, then navigation A is B's
parent task. And this is generalized to more than 2 tabs.

BUG= 707978 
R=zea@chromium.org

Review-Url: https://codereview.chromium.org/2868043003
Cr-Commit-Position: refs/heads/master@{#472667}

[modify] https://crrev.com/854b8bcfc787413293c86eddd979a3fd1936fb1a/components/sync_sessions/sessions_sync_manager.cc
[modify] https://crrev.com/854b8bcfc787413293c86eddd979a3fd1936fb1a/components/sync_sessions/sessions_sync_manager_unittest.cc
[modify] https://crrev.com/854b8bcfc787413293c86eddd979a3fd1936fb1a/components/sync_sessions/task_tracker.cc
[modify] https://crrev.com/854b8bcfc787413293c86eddd979a3fd1936fb1a/components/sync_sessions/task_tracker.h
[modify] https://crrev.com/854b8bcfc787413293c86eddd979a3fd1936fb1a/components/sync_sessions/task_tracker_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, May 23 2017

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

commit 7e3869bafd618a75247cd2082e6e72343425ec7e
Author: zea <zea@chromium.org>
Date: Tue May 23 02:51:51 2017

Revert of Track task ids for navigations cross multiple tabs. (patchset #4 id:80001 of https://codereview.chromium.org/2868043003/ )

Reason for revert:
crbug.com/725026 and crbug.com/724497

Original issue's description:
> Track task ids for navigations cross multiple tabs.
>
> This is a following step to generate task ids for navigations. Now we
> can track navigation relationship cross multiple tabs. If navigation B
> in tab2 is clicked from navigation A in tab1, then navigation A is B's
> parent task. And this is generalized to more than 2 tabs.
>
> BUG= 707978 
> R=zea@chromium.org
>
> Review-Url: https://codereview.chromium.org/2868043003
> Cr-Commit-Position: refs/heads/master@{#472667}
> Committed: https://chromium.googlesource.com/chromium/src/+/854b8bcfc787413293c86eddd979a3fd1936fb1a

TBR=shenchao@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 707978 

Review-Url: https://codereview.chromium.org/2901533003
Cr-Commit-Position: refs/heads/master@{#473790}

[modify] https://crrev.com/7e3869bafd618a75247cd2082e6e72343425ec7e/components/sync_sessions/sessions_sync_manager.cc
[modify] https://crrev.com/7e3869bafd618a75247cd2082e6e72343425ec7e/components/sync_sessions/sessions_sync_manager_unittest.cc
[modify] https://crrev.com/7e3869bafd618a75247cd2082e6e72343425ec7e/components/sync_sessions/task_tracker.cc
[modify] https://crrev.com/7e3869bafd618a75247cd2082e6e72343425ec7e/components/sync_sessions/task_tracker.h
[modify] https://crrev.com/7e3869bafd618a75247cd2082e6e72343425ec7e/components/sync_sessions/task_tracker_unittest.cc

Comment 8 by zea@chromium.org, May 23 2017

Owner: zea@chromium.org
Reverted due to crashes. Will reland once it's clear the source of the crashes.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 26 2017

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

commit 82892d4c1aadf89a18395593f87e3a6ecb6e87e4
Author: Nicolas Zea <zea@chromium.org>
Date: Mon Jun 26 22:57:16 2017

[Sync] Reland cross-tab tracking for task ids.

This relands https://codereview.chromium.org/2868043003 with a significant
overhaul. Included in the changes are:
1. Rewrite of how tasks are tracked. A map of navigation ids is used now instead
of an array indexed by navigation index.
2. Support for backfilling task ids when restoring a tab.
3. Android should now record task ids as well.

Bug:  707978 
Change-Id: I862256c41e56768ead51dc63a14d2f8bd57a3071
Reviewed-on: https://chromium-review.googlesource.com/547017
Commit-Queue: Nicolas Zea <zea@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#482448}
[modify] https://crrev.com/82892d4c1aadf89a18395593f87e3a6ecb6e87e4/components/sync_sessions/sessions_sync_manager.cc
[modify] https://crrev.com/82892d4c1aadf89a18395593f87e3a6ecb6e87e4/components/sync_sessions/sessions_sync_manager.h
[modify] https://crrev.com/82892d4c1aadf89a18395593f87e3a6ecb6e87e4/components/sync_sessions/sessions_sync_manager_unittest.cc
[modify] https://crrev.com/82892d4c1aadf89a18395593f87e3a6ecb6e87e4/components/sync_sessions/task_tracker.cc
[modify] https://crrev.com/82892d4c1aadf89a18395593f87e3a6ecb6e87e4/components/sync_sessions/task_tracker.h
[modify] https://crrev.com/82892d4c1aadf89a18395593f87e3a6ecb6e87e4/components/sync_sessions/task_tracker_unittest.cc

Comment 10 by zea@chromium.org, Jun 27 2017

Blockedon: 736994

Comment 11 by zea@chromium.org, Jul 10 2017

Summary: Save task id and ancestor navigations via session sync to backend data storage (was: Save ancestor navigations via session sync to backend data storage )
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 12 2017

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

commit 3655c79972e62be02cc2d0cfac405a8864bf9003
Author: Nicolas Zea <zea@chromium.org>
Date: Wed Jul 12 22:10:20 2017

[Sync] Handle delayed source tab id

When a link is opened in a new tab, the source tab value might be delayed.
This change makes it so that if the source tab id is present, and the task
tracker doesn't have a matching parent id, it will overwrite the tasks for that
tab with the tasks from the source tab.

This fixes the case where the user deliberately opens a link in a new tab
via middle click.

Bug:  707978 
Change-Id: I74be1a3527a6cda0187d7eb24477d83b57d2adbe
Reviewed-on: https://chromium-review.googlesource.com/567649
Commit-Queue: Nicolas Zea <zea@chromium.org>
Reviewed-by: Sky Malice <skym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486131}
[modify] https://crrev.com/3655c79972e62be02cc2d0cfac405a8864bf9003/components/sync_sessions/task_tracker.cc
[modify] https://crrev.com/3655c79972e62be02cc2d0cfac405a8864bf9003/components/sync_sessions/task_tracker.h
[modify] https://crrev.com/3655c79972e62be02cc2d0cfac405a8864bf9003/components/sync_sessions/task_tracker_unittest.cc

Comment 13 by zea@chromium.org, Feb 6 2018

Status: Fixed (was: Assigned)

Sign in to add a comment