Remove old webui History page on desktop and mobile

Remove old webui History page on desktop and mobile

This CL also drops a few features and flags relating to enabling:

1) native history on Android
2) material design history on desktop
3) grouped history (was not ported to MD history)

and generally audits strings and resources and stuff.

R=twellington@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2830983005
Cr-Commit-Position: refs/heads/master@{#469716}
Committed: https://chromium.googlesource.com/chromium/src/+/25b548f921b2eb2e255ddb3fe5b7e40c556e4363

[Dan Beam, 2017-04-20 23:52:27]

Description was changed from

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org,tsergeant@chromium.org,calamity@chromium.org
BUG=696821
==========

to

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org,tsergeant@chromium.org,calamity@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-21 00:01:07]

Description was changed from

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org,tsergeant@chromium.org,calamity@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove old webui History page on desktop and mobile

FUTURE_REVIEWERS=twellington@chromium.org,tsergeant@chromium.org,calamity@chr...
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-21 00:01:07]

dbeam@chromium.org changed reviewers:
- calamity@chromium.org, tsergeant@chromium.org, twellington@chromium.org

[Dan Beam, 2017-04-21 00:01:36]

Patchset #1 (id:1) has been deleted

[Dan Beam, 2017-04-21 00:18:03]

Patchset #1 (id:20001) has been deleted

[Dan Beam, 2017-04-21 00:18:22]

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

[commit-bot: I haz the power, 2017-04-21 00:18:45]

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-04-21 00:20:44]

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

[commit-bot: I haz the power, 2017-04-21 00:20:45]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Dan Beam, 2017-04-21 00:29:35]

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

[commit-bot: I haz the power, 2017-04-21 00:29:57]

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-04-21 00:32:42]

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

[commit-bot: I haz the power, 2017-04-21 00:32:43]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Dan Beam, 2017-04-25 23:22:09]

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

[commit-bot: I haz the power, 2017-04-25 23:22:57]

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-04-25 23:27:27]

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

[commit-bot: I haz the power, 2017-04-25 23:27:28]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-04-25 23:41:11]

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

[commit-bot: I haz the power, 2017-04-25 23:42:05]

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-04-25 23:53:59]

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

[commit-bot: I haz the power, 2017-04-25 23:54:00]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Dan Beam, 2017-04-26 00:08:04]

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

[commit-bot: I haz the power, 2017-04-26 00:08:40]

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-04-26 00:39:22]

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

[commit-bot: I haz the power, 2017-04-26 00:39:23]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Dan Beam, 2017-04-26 03:13:56]

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

[commit-bot: I haz the power, 2017-04-26 03:14:28]

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-04-26 03:48:40]

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

[commit-bot: I haz the power, 2017-04-26 03:48:41]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Dan Beam, 2017-04-26 04:24:56]

Patchset #7 (id:150001) has been deleted

[Dan Beam, 2017-04-26 04:25:05]

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

[commit-bot: I haz the power, 2017-04-26 04:25:25]

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-04-26 05:22:53]

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

[commit-bot: I haz the power, 2017-04-26 05:22:54]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Dan Beam, 2017-04-26 05:26:59]

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

[commit-bot: I haz the power, 2017-04-26 05:27:24]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-04-26 05:27:42]

Description was changed from

==========
Remove old webui History page on desktop and mobile

FUTURE_REVIEWERS=twellington@chromium.org,tsergeant@chromium.org,calamity@chr...
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org
FUTURE_REVIEWERS=tsergeant@chromium.org,calamity@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-26 05:27:43]

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

[Dan Beam, 2017-04-26 05:31:43]

twellington@: I'm getting closer to this CL working, so let's start the
reviewing.

i've got most (all?) platforms compiling, but my local .apk is crashing when I
try to view the history page or visit chrome://history directly on Android.  any
idea as to why?

I think it's likely to do with URL redirection from chrome://history ->
chrome-native://history.

any ideas which part of this I might not have updated (or updated incorrectly)?

[commit-bot: I haz the power, 2017-04-26 07:08:50]

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

[commit-bot: I haz the power, 2017-04-26 07:08:51]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Theresa, 2017-04-26 18:15:02]

On 2017/04/26 05:31:43, Dan Beam wrote:
> twellington@: I'm getting closer to this CL working, so let's start the
> reviewing.
> 
> i've got most (all?) platforms compiling, but my local .apk is crashing when I
> try to view the history page or visit chrome://history directly on Android. 
any
> idea as to why?
> 
> I think it's likely to do with URL redirection from chrome://history ->
> chrome-native://history.
> 
> any ideas which part of this I might not have updated (or updated
incorrectly)?

What's the crash?

There is redirect code in chrome_serialized_navigation_driver.cc,
new_tab_page_url_handler.cc, and browser_about_handler.cc, so it could be any of
those pieces. I would try adding history frame back to the conditionals for
redirect.

[Theresa, 2017-04-26 18:18:09]

https://codereview.chromium.org/2830983005/diff/190001/chrome/browser/android...
File chrome/browser/android/chrome_feature_list.cc (left):

https://codereview.chromium.org/2830983005/diff/190001/chrome/browser/android...
chrome/browser/android/chrome_feature_list.cc:43:
&features::kNativeAndroidHistoryManager,
Java is still using this parameter. We need to remove
HistoryManagerUtils#isAndroidHistoryManagerEnabled() (or change it to return
true).

We should also delete HistoryUiTest.java since that's testing the old history
UI.

fieldtrial_testing_config.json should also be cleaned up to remove the
AndroidHistoryManager config.

[Dan Beam, 2017-04-27 00:31:45]

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

[commit-bot: I haz the power, 2017-04-27 00:33:24]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-04-27 00:38:07]

https://codereview.chromium.org/2830983005/diff/190001/chrome/browser/android...
File chrome/browser/android/chrome_feature_list.cc (left):

https://codereview.chromium.org/2830983005/diff/190001/chrome/browser/android...
chrome/browser/android/chrome_feature_list.cc:43:
&features::kNativeAndroidHistoryManager,
On 2017/04/26 18:18:08, Theresa wrote:
> Java is still using this parameter. We need to remove
> HistoryManagerUtils#isAndroidHistoryManagerEnabled() (or change it to return
> true).

Done.

> 
> We should also delete HistoryUiTest.java since that's testing the old history
> UI.

Done.

> 
> fieldtrial_testing_config.json should also be cleaned up to remove the
> AndroidHistoryManager config.

Done.

https://codereview.chromium.org/2830983005/diff/210001/chrome/android/javates...
File chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java
(left):

https://codereview.chromium.org/2830983005/diff/210001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java:155:
loadUrl(mTestServer.getURL("/chrome/test/data/android/get_title_test.html"));
i'm fairly sure these files are re-used elsewhere

[commit-bot: I haz the power, 2017-04-27 00:38:47]

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

[commit-bot: I haz the power, 2017-04-27 00:38:48]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2017-04-27 03:17:14]

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

[commit-bot: I haz the power, 2017-04-27 03:18:17]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-04-27 03:36:49]

Description was changed from

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org
FUTURE_REVIEWERS=tsergeant@chromium.org,calamity@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-27 03:37:14]

/cc tsergeant@ and calamity@: feedback welcome, just trying to avoid adding too
many reviewers

[commit-bot: I haz the power, 2017-04-27 04:48:13]

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

[commit-bot: I haz the power, 2017-04-27 04:48:14]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Dan Beam, 2017-04-27 06:38:56]

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

[commit-bot: I haz the power, 2017-04-27 06:39:07]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-04-27 07:14:21]

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

[commit-bot: I haz the power, 2017-04-27 07:14:33]

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-04-27 08:14:10]

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

[commit-bot: I haz the power, 2017-04-27 08:14:11]

Dry run: This issue passed the CQ dry run.

[Theresa, 2017-04-27 17:38:23]

Android stuff lgtm. My PM will be evaluating launch data in about a week. It's
most prudent to wait to land till then, but if this needs to land now then
charge ahead.

https://codereview.chromium.org/2830983005/diff/210001/chrome/android/javates...
File chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java
(left):

https://codereview.chromium.org/2830983005/diff/210001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/HistoryUITest.java:155:
loadUrl(mTestServer.getURL("/chrome/test/data/android/get_title_test.html"));
On 2017/04/27 00:38:06, Dan Beam wrote:
> i'm fairly sure these files are re-used elsewhere

They most certainly are.

[Dan Beam, 2017-04-27 20:48:18]

Description was changed from

==========
Remove old webui History page on desktop and mobile

R=twellington@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove old webui History page on desktop and mobile

This CL also drops a few features and flags relating to enabling:

1) native history on Android
2) material design history on desktop
3) grouped history (was not ported to MD history)

and generally audits strings and resources and stuff.

R=twellington@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[Dan Beam, 2017-04-27 20:50:33]

dbeam@chromium.org changed reviewers:
+ asvitkine@chromium.org, jam@chromium.org, sdefresne@chromium.org,
sky@chromium.org, tedchoc@chromium.org

[Dan Beam, 2017-04-27 20:50:34]

alrighty, let's start up the big review machine:

+asvitkine@ for testings/variations/

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

+sdefresne@ for components/ and ios/

+tedchoc@ for chrome/android/, chrome/browser/android/

+sky@ for chrome/browser/ leftovers:

    chrome/browser/browser_about_handler.cc
    chrome/browser/history/history_browsertest.cc
    chrome/browser/sessions/better_session_restore_browsertest.cc
    chrome/browser/sessions/chrome_serialized_navigation_driver.cc
    chrome/browser/ui/browser_navigator_browsertest.cc
    chrome/browser/ui/find_bar/find_bar_host_browsertest.cc
    chrome/browser/ui/history_ui.cc
    chrome/browser/ui/history_ui.h

On 2017/04/27 17:38:23, Theresa wrote:
> Android stuff lgtm. My PM will be evaluating launch data in about a week. It's
> most prudent to wait to land till then, but if this needs to land now then
> charge ahead.

yep, fine with waiting to /land/ until then.

[sky, 2017-04-27 22:41:04]

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
File chrome/browser/browser_about_handler.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
chrome/browser/browser_about_handler.cc:92: // See http://crbug.com/654071.
Has native android history launcheD?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
File chrome/browser/browser_about_handler.cc (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
chrome/browser/browser_about_handler.cc:86: #if !defined(OS_ANDROID)
Move the ifdef up a line?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
File chrome/browser/history/history_browsertest.cc (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
chrome/browser/history/history_browsertest.cc:285:
IN_PROC_BROWSER_TEST_F(HistoryBrowserTest, DISABLED_HistorySearchXSS) {
How come this is being disabled everywhere not? Also, the comment on the
previous line is now out of date.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
File chrome/browser/sessions/better_session_restore_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:503: // Test that
switching MD History on behaves correctly with session restore.
Is there already a test that ensures the old history urls are mapped to the new?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/find...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:316: // TODO(tsergeant):
Get this test working on MD History, which loads very
Is it not useful to try to update this test for md history?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/hist...
File chrome/browser/ui/history_ui.h (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/hist...
chrome/browser/ui/history_ui.h:1: // Copyright 2017 The Chromium Authors. All
rights reserved.
I like keeping this closer to where it is used. How come you want to move it out
of webui?

[jam, 2017-04-28 00:25:18]

lgtm

[Dan Beam, 2017-04-28 03:17:45]

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
File chrome/browser/browser_about_handler.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
chrome/browser/browser_about_handler.cc:92: // See http://crbug.com/654071.
On 2017/04/27 22:41:04, sky wrote:
> Has native android history launcheD?

it's going to stable in M58 as we speak :D

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
File chrome/browser/browser_about_handler.cc (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/browser...
chrome/browser/browser_about_handler.cc:86: #if !defined(OS_ANDROID)
On 2017/04/27 22:41:04, sky wrote:
> Move the ifdef up a line?

Done.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
File chrome/browser/history/history_browsertest.cc (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
chrome/browser/history/history_browsertest.cc:285:
IN_PROC_BROWSER_TEST_F(HistoryBrowserTest, DISABLED_HistorySearchXSS) {
On 2017/04/27 22:41:04, sky wrote:
> How come this is being disabled everywhere not? Also, the comment on the
> previous line is now out of date.

why are the comments out of date?  they apply to why this was previously
disabled because of flakiness.

it's now being disabled because the new page loads in far more complex way. 
blocking on loading in a lazily-loaded polymer page is a little more involved,
and I consulted calamity@ and tsergeant@ over chat to ensure this was the right
thing to do for this test.

also, for what it's worth: it was only possible that this XSS problem exhibits
when using .innerHTML, which the new page does not (so I'm basically just
disabling a totallly precautionary regression test that's flaky).

are you OK with that?  should I add a TODO?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
File chrome/browser/sessions/better_session_restore_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:503: // Test that
switching MD History on behaves correctly with session restore.
On 2017/04/27 22:41:04, sky wrote:
> Is there already a test that ensures the old history urls are mapped to the
new?

I assume the main entry point to history is Chrome app menu -> History or
Ctrl/Cmd+h.

You probably already know this, but: the URL I'm removing (history-frame) is
kinda of hidden on desktop.  when you navigate to history on desktop, it shows
chrome://history.  behind the covers, it's actually:

  top frame: chrome://chrome/history
    left nav frame: chrome://uber-frame
    right nav frame: chrome://history-frame <-- I'm removing this

so the user never sees the "history-frame" URL unless they type it directly. 
bookmarks would be on "chrome://history".

on ChromeOS, the URL visited in a window is chrome://history-frame but the URL
bar is hidden.

the URL does kinda show on mobile, until you scroll (of course) and again, the
main entry point I'd imagine (due to how annoying and error-prone it is to type)
is Chrome app menu -> History.


On legacy URLs:

In settings, we changed some URLs.  We updated all the places in native UI that
these URLs were used (i.e. native UI opens a tab to
chrome://settings/someSubpage that's now chrome://settings/some/subpage).  We
did not attempt to keep backward compatible URLs forever.  I don't really see a
use to that.  The only things affected are maybe bookmarks or users that type
the URL, and those can both be updated.

As far as I know, no help center articles use chrome:// URLs.  Instead they use
a set of instructions specific to the UI (show the X, scroll to the Y, click the
Z), which also needs to be updated.

I think chrome:// URLs are weird.  They're internal.  We don't let people click
them by default, and they're mainly just echoing internal state to the user.  If
we could hide them, I'd be happier.


tl;dr - i don't think it's worth continuing to hold on to URLs forever for back
compatibility.  do you disagree?  maybe hold on them longer?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/find...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:316: // TODO(tsergeant):
Get this test working on MD History, which loads very
On 2017/04/27 22:41:04, sky wrote:
> Is it not useful to try to update this test for md history?

i just think it's a very unclear test and isn't buying us much.

it's doing all kinds of unrelated things.  together... in the same test case. 
without any form of isolation (i.e. bunch of systems and processes talking to
eachother, a bunch of assumptions).

which is probably why it's been flaky and half-disabled for 2.5 years.

do you need me to bring it back?

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/hist...
File chrome/browser/ui/history_ui.h (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/hist...
chrome/browser/ui/history_ui.h:1: // Copyright 2017 The Chromium Authors. All
rights reserved.
On 2017/04/27 22:41:04, sky wrote:
> I like keeping this closer to where it is used. How come you want to move it
out
> of webui?

because the Android _native_ UI also needs a favicon, and is not a "web" UI. 
but they're both parts of Chrome's browser UI, so I moved it to
chrome/browser/ui/.

[Dan Beam, 2017-04-28 03:17:54]

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

[commit-bot: I haz the power, 2017-04-28 03:18:31]

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-04-28 04:32:39]

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

[commit-bot: I haz the power, 2017-04-28 04:32:40]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2017-04-28 10:11:00]

ios & components lgtm (iOS stopped using webui history many revisions ago, so
should be a no-op there).

[Alexei Svitkine (slow), 2017-04-28 14:39:37]

LGTM!

[sky, 2017-04-28 17:43:54]

LGTM with improved comments as mentioned below. Also, I'm assuming old urls
aren't an issue.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
File chrome/browser/history/history_browsertest.cc (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
chrome/browser/history/history_browsertest.cc:285:
IN_PROC_BROWSER_TEST_F(HistoryBrowserTest, DISABLED_HistorySearchXSS) {
On 2017/04/28 03:17:45, Dan Beam wrote:
> On 2017/04/27 22:41:04, sky wrote:
> > How come this is being disabled everywhere not? Also, the comment on the
> > previous line is now out of date.
> 
> why are the comments out of date?  they apply to why this was previously
> disabled because of flakiness.
> 
> it's now being disabled because the new page loads in far more complex way. 
> blocking on loading in a lazily-loaded polymer page is a little more involved,
> and I consulted calamity@ and tsergeant@ over chat to ensure this was the
right
> thing to do for this test.
> 
> also, for what it's worth: it was only possible that this XSS problem exhibits
> when using .innerHTML, which the new page does not (so I'm basically just
> disabling a totallly precautionary regression test that's flaky).
> 
> are you OK with that?  should I add a TODO?

Before your change it was easy to correlate the comment with the code (eg linux
comment and OS_LINUX, windows comments and OS_WIN). Now the test is disabled
everywhere and there is no comment about mac. So, yes, please update the
comment. I would hope there is a single bug you can refer to that covers all
OSs, but you know better than I.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
File chrome/browser/sessions/better_session_restore_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:503: // Test that
switching MD History on behaves correctly with session restore.
On 2017/04/28 03:17:45, Dan Beam wrote:
> On 2017/04/27 22:41:04, sky wrote:
> > Is there already a test that ensures the old history urls are mapped to the
> new?
> 
> I assume the main entry point to history is Chrome app menu -> History or
> Ctrl/Cmd+h.
> 
> You probably already know this, but: the URL I'm removing (history-frame) is
> kinda of hidden on desktop.  when you navigate to history on desktop, it shows
> chrome://history.  behind the covers, it's actually:
> 
>   top frame: chrome://chrome/history
>     left nav frame: chrome://uber-frame
>     right nav frame: chrome://history-frame <-- I'm removing this
> 
> so the user never sees the "history-frame" URL unless they type it directly. 
> bookmarks would be on "chrome://history".
> 
> on ChromeOS, the URL visited in a window is chrome://history-frame but the URL
> bar is hidden.
> 
> the URL does kinda show on mobile, until you scroll (of course) and again, the
> main entry point I'd imagine (due to how annoying and error-prone it is to
type)
> is Chrome app menu -> History.
> 
> 
> On legacy URLs:
> 
> In settings, we changed some URLs.  We updated all the places in native UI
that
> these URLs were used (i.e. native UI opens a tab to
> chrome://settings/someSubpage that's now chrome://settings/some/subpage).  We
> did not attempt to keep backward compatible URLs forever.  I don't really see
a
> use to that.  The only things affected are maybe bookmarks or users that type
> the URL, and those can both be updated.
> 
> As far as I know, no help center articles use chrome:// URLs.  Instead they
use
> a set of instructions specific to the UI (show the X, scroll to the Y, click
the
> Z), which also needs to be updated.
> 
> I think chrome:// URLs are weird.  They're internal.  We don't let people
click
> them by default, and they're mainly just echoing internal state to the user. 
If
> we could hide them, I'd be happier.
> 
> 
> tl;dr - i don't think it's worth continuing to hold on to URLs forever for
back
> compatibility.  do you disagree?  maybe hold on them longer?

I was specifically thinking of an old URL entering the system via session
restore, or history. If you say that isn't possible, or doesn't cause problems.
Ok.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/find...
File chrome/browser/ui/find_bar/find_bar_host_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/find...
chrome/browser/ui/find_bar/find_bar_host_browsertest.cc:316: // TODO(tsergeant):
Get this test working on MD History, which loads very
On 2017/04/28 03:17:45, Dan Beam wrote:
> On 2017/04/27 22:41:04, sky wrote:
> > Is it not useful to try to update this test for md history?
> 
> i just think it's a very unclear test and isn't buying us much.
> 
> it's doing all kinds of unrelated things.  together... in the same test case. 
> without any form of isolation (i.e. bunch of systems and processes talking to
> eachother, a bunch of assumptions).
> 
> which is probably why it's been flaky and half-disabled for 2.5 years.
> 
> do you need me to bring it back?

Fair enough. Nuke it.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/hist...
File chrome/browser/ui/history_ui.h (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/ui/hist...
chrome/browser/ui/history_ui.h:1: // Copyright 2017 The Chromium Authors. All
rights reserved.
On 2017/04/28 03:17:45, Dan Beam wrote:
> On 2017/04/27 22:41:04, sky wrote:
> > I like keeping this closer to where it is used. How come you want to move it
> out
> > of webui?
> 
> because the Android _native_ UI also needs a favicon, and is not a "web" UI. 
> but they're both parts of Chrome's browser UI, so I moved it to
> chrome/browser/ui/.

Ok, make sense.

[Ted C, 2017-04-28 18:00:10]

lgtm

We might need to fix a couple other tests to (maybe just changing to
chrome://credits):

https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu...

https://cs.chromium.org/chromium/src/chrome/android/javatests/src/org/chromiu...

https://codereview.chromium.org/2830983005/diff/270001/chrome/browser/android...
File chrome/browser/android/ntp/new_tab_page_url_handler.cc (left):

https://codereview.chromium.org/2830983005/diff/270001/chrome/browser/android...
chrome/browser/android/ntp/new_tab_page_url_handler.cc:40: url->host() ==
kChromeUIHistoryFrameHost)) {
@twellington - I "think" we need to keep kChromeUIHistoryFrameHost here.  I
think if we remove this than any old history users will not get anything (where
we want to send them to native history instead).

57 is not a distant memory by any stretch on Android sadly.

https://codereview.chromium.org/2830983005/diff/270001/chrome/common/url_cons...
File chrome/common/url_constants.cc (left):

https://codereview.chromium.org/2830983005/diff/270001/chrome/common/url_cons...
chrome/common/url_constants.cc:64: const char kChromeUIHistoryFrameURL[] =
"chrome://history-frame/";
maybe just inline this in the one place that is still needed on android for
legacy reasons?

[Theresa, 2017-04-28 18:06:43]

https://codereview.chromium.org/2830983005/diff/270001/chrome/browser/android...
File chrome/browser/android/ntp/new_tab_page_url_handler.cc (left):

https://codereview.chromium.org/2830983005/diff/270001/chrome/browser/android...
chrome/browser/android/ntp/new_tab_page_url_handler.cc:40: url->host() ==
kChromeUIHistoryFrameHost)) {
On 2017/04/28 18:00:10, Ted C wrote:
> @twellington - I "think" we need to keep kChromeUIHistoryFrameHost here.  I
> think if we remove this than any old history users will not get anything
(where
> we want to send them to native history instead).
> 
> 57 is not a distant memory by any stretch on Android sadly.

That makes sense. If a user has a chrome://history-frame when they upgrade this
will probably still be hit. Safer to add it back.

[Dan Beam, 2017-04-29 02:39:43]

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
File chrome/browser/history/history_browsertest.cc (right):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/history...
chrome/browser/history/history_browsertest.cc:285:
IN_PROC_BROWSER_TEST_F(HistoryBrowserTest, DISABLED_HistorySearchXSS) {
On 2017/04/28 17:43:53, sky wrote:
> On 2017/04/28 03:17:45, Dan Beam wrote:
> > On 2017/04/27 22:41:04, sky wrote:
> > > How come this is being disabled everywhere not? Also, the comment on the
> > > previous line is now out of date.
> > 
> > why are the comments out of date?  they apply to why this was previously
> > disabled because of flakiness.
> > 
> > it's now being disabled because the new page loads in far more complex way. 
> > blocking on loading in a lazily-loaded polymer page is a little more
involved,
> > and I consulted calamity@ and tsergeant@ over chat to ensure this was the
> right
> > thing to do for this test.
> > 
> > also, for what it's worth: it was only possible that this XSS problem
exhibits
> > when using .innerHTML, which the new page does not (so I'm basically just
> > disabling a totallly precautionary regression test that's flaky).
> > 
> > are you OK with that?  should I add a TODO?
> 
> Before your change it was easy to correlate the comment with the code (eg
linux
> comment and OS_LINUX, windows comments and OS_WIN). Now the test is disabled
> everywhere and there is no comment about mac. So, yes, please update the
> comment. I would hope there is a single bug you can refer to that covers all
> OSs, but you know better than I.

Done.

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
File chrome/browser/sessions/better_session_restore_browsertest.cc (left):

https://codereview.chromium.org/2830983005/diff/250001/chrome/browser/session...
chrome/browser/sessions/better_session_restore_browsertest.cc:503: // Test that
switching MD History on behaves correctly with session restore.
On 2017/04/28 17:43:54, sky wrote:
> On 2017/04/28 03:17:45, Dan Beam wrote:
> > On 2017/04/27 22:41:04, sky wrote:
> > > Is there already a test that ensures the old history urls are mapped to
the
> > new?
> > 
> > I assume the main entry point to history is Chrome app menu -> History or
> > Ctrl/Cmd+h.
> > 
> > You probably already know this, but: the URL I'm removing (history-frame) is
> > kinda of hidden on desktop.  when you navigate to history on desktop, it
shows
> > chrome://history.  behind the covers, it's actually:
> > 
> >   top frame: chrome://chrome/history
> >     left nav frame: chrome://uber-frame
> >     right nav frame: chrome://history-frame <-- I'm removing this
> > 
> > so the user never sees the "history-frame" URL unless they type it directly.

> > bookmarks would be on "chrome://history".
> > 
> > on ChromeOS, the URL visited in a window is chrome://history-frame but the
URL
> > bar is hidden.
> > 
> > the URL does kinda show on mobile, until you scroll (of course) and again,
the
> > main entry point I'd imagine (due to how annoying and error-prone it is to
> type)
> > is Chrome app menu -> History.
> > 
> > 
> > On legacy URLs:
> > 
> > In settings, we changed some URLs.  We updated all the places in native UI
> that
> > these URLs were used (i.e. native UI opens a tab to
> > chrome://settings/someSubpage that's now chrome://settings/some/subpage). 
We
> > did not attempt to keep backward compatible URLs forever.  I don't really
see
> a
> > use to that.  The only things affected are maybe bookmarks or users that
type
> > the URL, and those can both be updated.
> > 
> > As far as I know, no help center articles use chrome:// URLs.  Instead they
> use
> > a set of instructions specific to the UI (show the X, scroll to the Y, click
> the
> > Z), which also needs to be updated.
> > 
> > I think chrome:// URLs are weird.  They're internal.  We don't let people
> click
> > them by default, and they're mainly just echoing internal state to the user.

> If
> > we could hide them, I'd be happier.
> > 
> > 
> > tl;dr - i don't think it's worth continuing to hold on to URLs forever for
> back
> > compatibility.  do you disagree?  maybe hold on them longer?
> 
> I was specifically thinking of an old URL entering the system via session
> restore, or history. If you say that isn't possible, or doesn't cause
problems.
> Ok.

it's possible in some cases, so i restored some legacy redirection for now

https://codereview.chromium.org/2830983005/diff/270001/chrome/browser/android...
File chrome/browser/android/ntp/new_tab_page_url_handler.cc (left):

https://codereview.chromium.org/2830983005/diff/270001/chrome/browser/android...
chrome/browser/android/ntp/new_tab_page_url_handler.cc:40: url->host() ==
kChromeUIHistoryFrameHost)) {
On 2017/04/28 18:06:43, Theresa wrote:
> On 2017/04/28 18:00:10, Ted C wrote:
> > @twellington - I "think" we need to keep kChromeUIHistoryFrameHost here.  I
> > think if we remove this than any old history users will not get anything
> (where
> > we want to send them to native history instead).
> > 
> > 57 is not a distant memory by any stretch on Android sadly.
> 
> That makes sense. If a user has a chrome://history-frame when they upgrade
this
> will probably still be hit. Safer to add it back.

Done.

[Dan Beam, 2017-04-29 02:40:04]

Patchset #13 (id:290001) has been deleted

[Dan Beam, 2017-04-29 02:40:14]

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

[commit-bot: I haz the power, 2017-04-29 02:40:33]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-04-29 02:56:16]

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

[commit-bot: I haz the power, 2017-04-29 02:56:28]

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-04-29 03:22:21]

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

[commit-bot: I haz the power, 2017-04-29 03:22:23]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Dan Beam, 2017-05-01 21:06:02]

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

[commit-bot: I haz the power, 2017-05-01 21:06:56]

Dry run: CQ is trying da patch.

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

[Dan Beam, 2017-05-01 21:16:10]

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

[commit-bot: I haz the power, 2017-05-01 21:17:14]

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-05-01 22:37:01]

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

[commit-bot: I haz the power, 2017-05-01 22:37:02]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-05-03 01:32:39]

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

[commit-bot: I haz the power, 2017-05-03 01:33:27]

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-05-03 03:15:03]

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

[commit-bot: I haz the power, 2017-05-03 03:15:05]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-05-04 18:50:59]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2017-05-04 18:51:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, jam@chromium.org, sky@chromium.org,
tedchoc@chromium.org, asvitkine@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2830983005/#ps370001
(title: "merge")

[commit-bot: I haz the power, 2017-05-04 18:51:51]

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-05-04 18:57:27]

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

[commit-bot: I haz the power, 2017-05-04 18:57:30]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Dan Beam, 2017-05-04 19:22:51]

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

[commit-bot: I haz the power, 2017-05-04 19:23:22]

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-05-04 19:28:51]

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

[commit-bot: I haz the power, 2017-05-04 19:28:54]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[Dan Beam, 2017-05-05 00:05:45]

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

[commit-bot: I haz the power, 2017-05-05 00:06: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-05-05 01:29:22]

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

[commit-bot: I haz the power, 2017-05-05 01:29:24]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-05-05 17:41:01]

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

[commit-bot: I haz the power, 2017-05-05 17:41:35]

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-05-05 17:50:27]

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

[commit-bot: I haz the power, 2017-05-05 17:50:30]

Dry run: This issue passed the CQ dry run.

[Dan Beam, 2017-05-05 17:53:39]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2017-05-05 17:53:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, sky@chromium.org, jam@chromium.org,
tedchoc@chromium.org, asvitkine@chromium.org, sdefresne@chromium.org
Link to the patchset: https://codereview.chromium.org/2830983005/#ps410001
(title: "merge")

[commit-bot: I haz the power, 2017-05-05 17:54:19]

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-05-05 18:05:28]

CQ is committing da patch.

Bot data: {"patchset_id": 410001, "attempt_start_ts": 1494006819544640,
"parent_rev": "a5888e81e68aff997094e675cccfe09c559d5f4b", "commit_rev":
"25b548f921b2eb2e255ddb3fe5b7e40c556e4363"}

[commit-bot: I haz the power, 2017-05-05 18:05:43]

Description was changed from

==========
Remove old webui History page on desktop and mobile

This CL also drops a few features and flags relating to enabling:

1) native history on Android
2) material design history on desktop
3) grouped history (was not ported to MD history)

and generally audits strings and resources and stuff.

R=twellington@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Remove old webui History page on desktop and mobile

This CL also drops a few features and flags relating to enabling:

1) native history on Android
2) material design history on desktop
3) grouped history (was not ported to MD history)

and generally audits strings and resources and stuff.

R=twellington@chromium.org
BUG=696821
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2830983005
Cr-Commit-Position: refs/heads/master@{#469716}
Committed:
https://chromium.googlesource.com/chromium/src/+/25b548f921b2eb2e255ddb3fe5b7...
==========

[commit-bot: I haz the power, 2017-05-05 18:05:49]

Committed patchset #18 (id:410001) as
https://chromium.googlesource.com/chromium/src/+/25b548f921b2eb2e255ddb3fe5b7...