MD History: re-use history host constant from content/ instead of duplicating to chrome/

MD History: re-use history host constant from content/ instead of duplicating to chrome/

R=calamity@chromium.org
BUG=425627
Review-Url: https://codereview.chromium.org/2751653004
Cr-Commit-Position: refs/heads/master@{#456951}
Committed: https://chromium.googlesource.com/chromium/src/+/e4d8d06349a9454d77ccdb1a2e2bdfdf085fd044

[Dan Beam, 2017-03-14 06:00:00]

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

[commit-bot: I haz the power, 2017-03-14 06:00:20]

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-03-14 06:00:20]

Description was changed from

==========
MD History: re-use URL constants from content/ instead of duplicating to chrome/

R=calamity@chromium.org
BUG=none
==========

to

==========
MD History: re-use history host constant from content/ instead of duplicating to
chrome/

R=calamity@chromium.org
BUG=none
==========

[commit-bot: I haz the power, 2017-03-14 06:05:42]

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

[commit-bot: I haz the power, 2017-03-14 06:05:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Dan Beam, 2017-03-14 06:09:09]

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

[commit-bot: I haz the power, 2017-03-14 06:09: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-03-14 06:15:20]

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

[commit-bot: I haz the power, 2017-03-14 06:15:21]

Dry run: Try jobs failed on following builders:
  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-03-14 06:18:29]

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

[commit-bot: I haz the power, 2017-03-14 06:18:50]

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-03-14 06:24:12]

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

[commit-bot: I haz the power, 2017-03-14 06:24:13]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[calamity, 2017-03-14 06:35:54]

lgtm, guess I did duplicate it. Thanks for cleaning.

[Dan Beam, 2017-03-14 06:39:07]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2017-03-14 06:39:34]

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-03-14 06:46:25]

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

[commit-bot: I haz the power, 2017-03-14 06:46:26]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Dan Beam, 2017-03-14 16:12:44]

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

[Dan Beam, 2017-03-14 16:12:45]

+thestig@ for

    chrome/browser/android/ntp/new_tab_page_url_handler.cc
    chrome/browser/browser_about_handler.cc
    chrome/browser/ui/browser_navigator.cc
    chrome/common/extensions/chrome_manifest_url_handlers.cc

[Lei Zhang, 2017-03-14 18:30:54]

Do you find it weird that content/ knows about history but only use it in 1
place? I guess what I'm really doing is giving a bit more pushback on
https://codereview.chromium.org/2355543003

[Lei Zhang, 2017-03-14 18:32:50]

Let me upload an alternative CL in a bit and let's see if that's any better.

[Lei Zhang, 2017-03-14 23:59:23]

On 2017/03/14 18:32:50, Lei Zhang (super slow) wrote:
> Let me upload an alternative CL in a bit and let's see if that's any better.

Taking a while because Android piled on more content::kChromeUIHistoryHost usage
in ContentSerializedNavigationDriver::Sanitize(). Thinking about this a bit
more, ContentSerializedNavigationDriver::Sanitize() is a component, but it's
being hard coded with a bunch of rules for higher level features that it really
shouldn't know about. So my proposal is to delegate most of Sanitize() to
chrome/.

[Dan Beam, 2017-03-15 00:16:25]

Description was changed from

==========
MD History: re-use history host constant from content/ instead of duplicating to
chrome/

R=calamity@chromium.org
BUG=none
==========

to

==========
MD History: re-use history host constant from content/ instead of duplicating to
chrome/

R=calamity@chromium.org
BUG=425627
==========

[Lei Zhang, 2017-03-15 00:17:07]

But if you want to do this for now to get the blocking CL to land, then LGTM. We
can reexamine which constants below where later.

[Dan Beam, 2017-03-15 00:17:21]

note: this is a dependent CL of

https://codereview.chromium.org/2743323005/

which allows users to restart chrome agnostic to the MaterialDesignSettings
feature.

[Dan Beam, 2017-03-15 00:17:55]

On 2017/03/15 00:17:07, Lei Zhang (super slow) wrote:
> But if you want to do this for now to get the blocking CL to land, then LGTM.
We
> can reexamine which constants below where later.

I'm happy to review a CL that re-uses these constants in a more crafty way

[Dan Beam, 2017-03-15 00:18:16]

The CQ bit was checked by dbeam@chromium.org

[commit-bot: I haz the power, 2017-03-15 00:18:44]

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

[Lei Zhang, 2017-03-15 00:24:28]

On 2017/03/14 23:59:23, Lei Zhang (super slow) wrote:
> On 2017/03/14 18:32:50, Lei Zhang (super slow) wrote:
> > Let me upload an alternative CL in a bit and let's see if that's any better.
> 
> Taking a while because Android piled on more content::kChromeUIHistoryHost
usage
> in ContentSerializedNavigationDriver::Sanitize(). Thinking about this a bit
> more, ContentSerializedNavigationDriver::Sanitize() is a component, but it's
> being hard coded with a bunch of rules for higher level features that it
really
> shouldn't know about. So my proposal is to delegate most of Sanitize() to
> chrome/.

Deferred to https://bugs.chromium.org/p/chromium/issues/detail?id=701605 and let
the components/sessions/OWNERS decide on which direction to go in.

[commit-bot: I haz the power, 2017-03-15 00:25:30]

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

[commit-bot: I haz the power, 2017-03-15 00:25:34]

Failed to apply patch for
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:
While running git apply --index -p1;
  error: patch failed:
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:80
  error: chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc: patch does
not apply

Patch:       chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
Index: chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
diff --git a/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
b/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
index
df5e8b387a6eb692b867c961469779ac4a791277..e32cc3a8c42e19d5f28e97a53d28f88214e0214a
100644
--- a/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
+++ b/chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc
@@ -80,6 +80,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
 #include "extensions/features/features.h"
 #include "media/media_features.h"
@@ -436,7 +437,7 @@ WebUIFactoryFunction GetWebUIFactoryFunction(WebUI* web_ui,
   }
   // Material Design history is on its own host, rather than on an Uber page.
   if (base::FeatureList::IsEnabled(features::kMaterialDesignHistory) &&
-      url.host_piece() == chrome::kChromeUIHistoryHost) {
+      url.host_piece() == content::kChromeUIHistoryHost) {
     return &NewWebUI<MdHistoryUI>;
   }
   // Material Design Settings gets its own host, if enabled.
@@ -795,7 +796,7 @@ base::RefCountedMemory*
ChromeWebUIControllerFactory::GetFaviconResourceBytes(
   if (page_url.host_piece() == chrome::kChromeUIFlagsHost)
     return FlagsUI::GetFaviconResourceBytes(scale_factor);
 
-  if (page_url.host_piece() == chrome::kChromeUIHistoryHost)
+  if (page_url.host_piece() == content::kChromeUIHistoryHost)
     return HistoryUI::GetFaviconResourceBytes(scale_factor);
 
 #if !defined(OS_ANDROID)

[Dan Beam, 2017-03-15 00:35:49]

The CQ bit was checked by dbeam@chromium.org

[Dan Beam, 2017-03-15 00:35:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
thestig@chromium.org, calamity@chromium.org
Link to the patchset: https://codereview.chromium.org/2751653004/#ps80001
(title: "merge")

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

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-03-15 02:12:25]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489538149760170,
"parent_rev": "2691c6929de42bf5845162fe9df31270c9b38b18", "commit_rev":
"e4d8d06349a9454d77ccdb1a2e2bdfdf085fd044"}

[commit-bot: I haz the power, 2017-03-15 02:13:08]

Description was changed from

==========
MD History: re-use history host constant from content/ instead of duplicating to
chrome/

R=calamity@chromium.org
BUG=425627
==========

to

==========
MD History: re-use history host constant from content/ instead of duplicating to
chrome/

R=calamity@chromium.org
BUG=425627

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

[commit-bot: I haz the power, 2017-03-15 02:13:09]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/e4d8d06349a9454d77ccdb1a2e2b...