New issue
Advanced search Search tips

Issue 859902 link

Starred by 1 user
Status: Assigned
Owner:
a...@chromium.org
Cc:
sky@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Does history service need "context id"?

Project Member Reported by a...@chromium.org, Jul 3 
HistoryAddPageArgs used to have both a ContextID and a page id. Page ids were unique within a WebContents, so a ContextID was generated for a WebContents (see history::ContextIDForWebContents), and the (ContextID, page id) tuple was used in History.

I killed page id years ago, and replaced its use in the history service with the NavigationEntry's unique id. That id is unique across all of Chrome during a session.

Yet the docs say that the id needs to be scoped by the context id. Now that the id is really globally unique per session, can we remove the concept of "context id"?
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 6 

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

commit 87b0ae4a09eef5c30e5771ce84f2789c40ae9b86
Author: Avi Drissman <avi@chromium.org>
Date: Fri Jul 06 15:09:20 2018

Add the page titles to the history service.

The history service assumed that the page would always explicitly
call "set title" when the title was set. However, when a page
does a pushState, the title stays set, and there is no explicit
set title call. Therefore, pass the current page title to the
history system when navigating same-document.

Also, add a bug reference to some old infrastructure that might
be obsolete.

BUG= 840663 , 859902
TEST=as in first bug, the new test in this CL

Cq-Include-Trybots: luci.chromium.try:ios-simulator-full-configs;master.tryserver.chromium.mac:ios-simulator-cronet
Change-Id: Ie5bc17239a20cc2e364daa4fd358f07996c907eb
Reviewed-on: https://chromium-review.googlesource.com/1125159
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572976}
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/chrome/browser/history/history_browsertest.cc
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/chrome/browser/history/history_tab_helper.cc
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/components/history/core/browser/history_backend.cc
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/components/history/core/browser/history_backend.h
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/components/history/core/browser/history_service.h
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/components/history/core/browser/history_types.cc
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/components/history/core/browser/history_types.h
[modify] https://crrev.com/87b0ae4a09eef5c30e5771ce84f2789c40ae9b86/ios/chrome/browser/history/history_tab_helper.mm

Comment 2 by benhenry@chromium.org, Aug 2

Status: Assigned (was: Available)
►

Sign in to add a comment