Switch the history backend from using page ids to navigation entry unique ids.

Switch the history backend from using page ids to navigation entry unique ids.

BUG=371816, 369661, 390683
TEST=existing unit tests

Committed: https://crrev.com/0800fee9ecd0c42e10189ae924cc4861fbea8cce
Cr-Commit-Position: refs/heads/master@{#309991}

[Avi (use Gerrit), 2014-12-26 22:13:47]

avi@chromium.org changed reviewers:
+ sdefresne@chromium.org, sky@chromium.org

[Avi (use Gerrit), 2014-12-26 22:13:47]

I'm trying to kill page ids, and this code is pretty much the only place that
they're still used outside of navigation. NavigationEntries have a unique id
value that is strictly increasing, and happens to be unique across the browser
process during execution. That makes those id values a good substitute for page
ids.

[sdefresne, 2014-12-29 09:59:39]

Does this means we can also remove the context_id?

https://codereview.chromium.org/823273003/diff/20001/components/history/core/...
File components/history/core/browser/history_types.h (right):

https://codereview.chromium.org/823273003/diff/20001/components/history/core/...
components/history/core/browser/history_types.h:376: //       GURL(),  /* url */
nit: I don't think this reformatting improves readability. The name and type of
the parameter can be easily seen from the second constructor that is defined on
the next line.

[Avi (use Gerrit), 2014-12-29 16:45:08]

On 2014/12/29 09:59:39, sdefresne wrote:
> Does this means we can also remove the context_id?

Well, yes, but.

Right now, the only way that items are removed from the VisitTracker is when the
tab is closed, and all items from the ContextID are removed. We could remove
ContextID, but we don't want the VisitTracker to grow forever, and there's no
good mechanism for listening for the destruction of NavigationEntries. So I
repurposed ContextID to be the _scope_ of a group of NavigationEntries. (Note
the rewording: "a context to scope page ids" -> "a context to scope the lifetime
of navigation entry references".)

It's the simplest change for now that removes page ids. If you want to revisit
the code to clean it up further, and if you have a good idea as to how to remove
ContextID, this change frees you to do so. As I don't have a good idea now,
however, I'm proposing this version.

[sdefresne, 2014-12-29 18:35:25]

lgtm

Thank you for the details about ContextID. I have a plan to change the type so
that it does not block the componentization. Just wanted to check whether it was
still required.

[sdefresne, 2014-12-29 18:35:51]

sdefresne@chromium.org changed reviewers:
+ brettw@chromium.org

[sdefresne, 2014-12-29 18:35:51]

+brettw who is the OWNERS of //chrome/browser/history

[Avi (use Gerrit), 2014-12-29 18:43:13]

On 2014/12/29 18:35:51, sdefresne wrote:
> +brettw who is the OWNERS of //chrome/browser/history

Also, don't forget http://crbug.com/390683, that page id as used today is
completely _wrong_ since it's scoped to RenderViewHost, not WebContents. This
change actually makes things correct! :)

[Avi (use Gerrit), 2014-12-29 18:43:21]

https://codereview.chromium.org/823273003/diff/20001/components/history/core/...
File components/history/core/browser/history_types.h (right):

https://codereview.chromium.org/823273003/diff/20001/components/history/core/...
components/history/core/browser/history_types.h:376: //       GURL(),  /* url */
On 2014/12/29 09:59:39, sdefresne wrote:
> nit: I don't think this reformatting improves readability. The name and type
of
> the parameter can be easily seen from the second constructor that is defined
on
> the next line.

Done.

[sky, 2015-01-05 17:14:12]

LGTM

[Avi (use Gerrit), 2015-01-05 19:48:55]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2015-01-05 19:50:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/823273003/60001

[commit-bot: I haz the power, 2015-01-05 23:21:25]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-01-05 23:22:27]

Patchset 4 (id:??) landed as
https://crrev.com/0800fee9ecd0c42e10189ae924cc4861fbea8cce
Cr-Commit-Position: refs/heads/master@{#309991}