[Android History] Fix chrome://history redirect

[Android History] Fix chrome://history redirect

BUG=685829
Review-Url: https://codereview.chromium.org/2676893002
Cr-Commit-Position: refs/heads/master@{#448642}
Committed: https://chromium.googlesource.com/chromium/src/+/a0d282eee1b4e20cd10f0bfaef744a331b1967fa

[Theresa, 2017-02-03 22:12:25]

twellington@chromium.org changed reviewers:
+ dfalcantara@chromium.org

[Theresa, 2017-02-03 22:12:25]

ptal - demo videos are in the bug

[Theresa, 2017-02-03 22:13:55]

The CQ bit was checked by twellington@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 22:14:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-03 23:04:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 23:04:16]

Dry run: This issue passed the CQ dry run.

[gone, 2017-02-06 18:47:20]

lgtm

[Theresa, 2017-02-06 18:57:24]

twellington@chromium.org changed reviewers:
+ nasko@chromium.org, sky@chromium.org

[Theresa, 2017-02-06 18:57:24]

+sky@ for components/sessions/content/content_serialized_navigation_driver.cc
and  chrome/browser/browser_about_handler.cc

+nasko@ for content/public/common/content_features.*

[nasko, 2017-02-06 19:15:09]

content/public/common/content_features.* LGTM

[sky, 2017-02-06 22:32:14]

I have a meta question about this. Did you consider making chrome:// redirect
automatically to chrome-native:// so that you don't have to patch all sites like
this? If not, how come?

https://codereview.chromium.org/2676893002/diff/1/components/sessions/content...
File components/sessions/content/content_serialized_navigation_driver.cc
(right):

https://codereview.chromium.org/2676893002/diff/1/components/sessions/content...
components/sessions/content/content_serialized_navigation_driver.cc:146:
navigation->virtual_url_.host_piece() == "history-frame")) {
Is history-frame really not defined anywhere?

https://codereview.chromium.org/2676893002/diff/1/components/sessions/content...
components/sessions/content/content_serialized_navigation_driver.cc:148:
navigation->virtual_url_ = GURL("chrome-native://history/");
Seems like this should be constants, right?

[Theresa, 2017-02-06 22:43:28]

On 2017/02/06 22:32:14, sky wrote:
> I have a meta question about this. Did you consider making chrome:// redirect
> automatically to chrome-native:// so that you don't have to patch all sites
like
> this? If not, how come?


chrome://history does redirect to chrome-native://history in
new_tab_page_url_handler.cc. This patch is to cover chrome://history tabs that
are open prior to upgrading to a version of Chrome with the new history manager.
When restoring tab state, we do redirect chrome://history to
chrome-native://history but RenderFrameImpl trashes the redirect and gets the
URL to load from the frame history (full stack/flow in the bug). There is
already logic ContentSerializedNavigationDriver to support similar redirects for
the Android NTP and for the redirect of old desktop chrome://history to the new
material design chrome://history, so this seemed like the correct place to
overwrite the serialized tab state info with the correct chrome-native URL.

[Theresa, 2017-02-06 23:38:07]

The CQ bit was checked by twellington@chromium.org to run a CQ dry run

[Theresa, 2017-02-06 23:38:09]

https://codereview.chromium.org/2676893002/diff/1/components/sessions/content...
File components/sessions/content/content_serialized_navigation_driver.cc
(right):

https://codereview.chromium.org/2676893002/diff/1/components/sessions/content...
components/sessions/content/content_serialized_navigation_driver.cc:146:
navigation->virtual_url_.host_piece() == "history-frame")) {
On 2017/02/06 22:32:14, sky wrote:
> Is history-frame really not defined anywhere?

It wasn't defined anywhere that was accessible by components/. I added it to
url_constants.h.

https://codereview.chromium.org/2676893002/diff/1/components/sessions/content...
components/sessions/content/content_serialized_navigation_driver.cc:148:
navigation->virtual_url_ = GURL("chrome-native://history/");
On 2017/02/06 22:32:14, sky wrote:
> Seems like this should be constants, right?

Done.

[commit-bot: I haz the power, 2017-02-06 23:39:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sky, 2017-02-07 00:23:48]

LGTM

[commit-bot: I haz the power, 2017-02-07 01:44:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-07 01:44:16]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-02-07 16:30:46]

The CQ bit was checked by twellington@chromium.org

[Theresa, 2017-02-07 16:30:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2676893002/#ps20001
(title: "[Android History] Fix chrome://history redirect")

[commit-bot: I haz the power, 2017-02-07 16:31:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-07 16:36:46]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486485046076860,
"parent_rev": "0268543786283db4107d2362f338901d909545b1", "commit_rev":
"a0d282eee1b4e20cd10f0bfaef744a331b1967fa"}

[commit-bot: I haz the power, 2017-02-07 16:37:19]

Description was changed from

==========
[Android History] Fix chrome://history redirect

BUG=685829
==========

to

==========
[Android History] Fix chrome://history redirect

BUG=685829

Review-Url: https://codereview.chromium.org/2676893002
Cr-Commit-Position: refs/heads/master@{#448642}
Committed:
https://chromium.googlesource.com/chromium/src/+/a0d282eee1b4e20cd10f0bfaef74...
==========

[commit-bot: I haz the power, 2017-02-07 16:37:20]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/a0d282eee1b4e20cd10f0bfaef74...

[Dan Beam, 2017-03-14 05:41:55]

dbeam@chromium.org changed reviewers:
+ dbeam@chromium.org

[Dan Beam, 2017-03-14 05:41:55]

https://codereview.chromium.org/2676893002/diff/20001/content/public/common/u...
File content/public/common/url_constants.cc (right):

https://codereview.chromium.org/2676893002/diff/20001/content/public/common/u...
content/public/common/url_constants.cc:74: const char kChromeUIHistoryURL[] =
"chrome://history/";
why do we need the chrome/common/url_constants.h version of this now that it's
in content/public/common/url_constants.h?

[Theresa, 2017-03-14 15:05:50]

https://codereview.chromium.org/2676893002/diff/20001/content/public/common/u...
File content/public/common/url_constants.cc (right):

https://codereview.chromium.org/2676893002/diff/20001/content/public/common/u...
content/public/common/url_constants.cc:74: const char kChromeUIHistoryURL[] =
"chrome://history/";
On 2017/03/14 05:41:55, Dan Beam wrote:
> why do we need the chrome/common/url_constants.h version of this now that it's
> in content/public/common/url_constants.h?

We probably don't. There are a number of duplicates across these two files that
could probably be cleaned up.