Add a basic stub Polymer settings page at chrome://md-settings.

Add a basic settings page with a Polymer paper element which loads at chrome://md-settings (if the enable-md-settings flag is enabled).

BUG=454961
Committed: https://crrev.com/91bc7932a4c33873515a2237dbf4ab933e16bb94
Cr-Commit-Position: refs/heads/master@{#314685}

[Oren Blasberg, 2015-02-04 01:51:58]

orenb@chromium.org changed reviewers:
+ jhawkins@chromium.org, stevenjb@chromium.org

[stevenjb, 2015-02-04 02:19:16]

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:16: <script
src="strings.js"></script>
I think we should consider starting with as blank a slate as possible initially,
i.e. just chrome_shared.css. Thoughts on that?

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:23: </style>
Iirc, we decided to separate .css from .html?

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:32: <script
src="chrome://resources/js/i18n_template2.js"></script>
Do we want to start out with the existing chrome i18n, or skip that and plan to
do something better "very soon"?

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:373: LOG(ERROR) <<
"url host is MD settings host";
VLOG (I know it's less convenient but we shouldn't check in ERROR for non
errors)

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:387: LOG(ERROR) <<
"url host is regular settings host";
And definitely not here :)

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/md_settings_ui.cc (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.cc:21: // 
html_source->SetUseJsonJSFormatV2();
elim or comment

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/md_settings_ui.h (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:9: #include "ui/base/layout.h"
not needed?

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:12: class RefCountedMemory;
not needed?

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:14: 
All classes should have a comment, even trivial ones

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:21: ui::ScaleFactor scale_factor);
elim

[Jeremy Klein, 2015-02-04 02:24:26]

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

[Jeremy Klein, 2015-02-04 02:24:26]

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:16: <script
src="strings.js"></script>
On 2015/02/04 02:19:15, stevenjb wrote:
> I think we should consider starting with as blank a slate as possible
initially,
> i.e. just chrome_shared.css. Thoughts on that?

+1 to clearing this out for now. Are any of the above js files still required?

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:32: <script
src="chrome://resources/js/i18n_template2.js"></script>
On 2015/02/04 02:19:15, stevenjb wrote:
> Do we want to start out with the existing chrome i18n, or skip that and plan
to
> do something better "very soon"?

I thought we were planning on using the chrome i18n solution still. Was there a
different option available?

[michaelpg, 2015-02-04 02:35:29]

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

[michaelpg, 2015-02-04 02:35:30]

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:5: <title
i18n-content="settings_title"></title>
settingsTitle: useCamelCase (what we use for JS variables)

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:32: <script
src="chrome://resources/js/i18n_template2.js"></script>
On 2015/02/04 02:19:15, stevenjb wrote:
> Do we want to start out with the existing chrome i18n, or skip that and plan
to
> do something better "very soon"?

I'd say skip it. We'll probably use chrome.i18n for the app.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:373: LOG(ERROR) <<
"url host is MD settings host";
On 2015/02/04 02:19:15, stevenjb wrote:
> VLOG (I know it's less convenient but we shouldn't check in ERROR for non
> errors)

I would just remove this logging altogether.

[Oren Blasberg, 2015-02-04 18:31:51]

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:5: <title
i18n-content="settings_title"></title>
On 2015/02/04 02:35:29, michaelpg wrote:
> settingsTitle: useCamelCase (what we use for JS variables)

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:16: <script
src="strings.js"></script>
On 2015/02/04 02:24:26, Jeremy Klein wrote:
> On 2015/02/04 02:19:15, stevenjb wrote:
> > I think we should consider starting with as blank a slate as possible
> initially,
> > i.e. just chrome_shared.css. Thoughts on that?
> 
> +1 to clearing this out for now. Are any of the above js files still required?

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:23: </style>
On 2015/02/04 02:19:15, stevenjb wrote:
> Iirc, we decided to separate .css from .html?

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:32: <script
src="chrome://resources/js/i18n_template2.js"></script>
On 2015/02/04 02:35:29, michaelpg wrote:
> On 2015/02/04 02:19:15, stevenjb wrote:
> > Do we want to start out with the existing chrome i18n, or skip that and plan
> to
> > do something better "very soon"?
> 
> I'd say skip it. We'll probably use chrome.i18n for the app.

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:373: LOG(ERROR) <<
"url host is MD settings host";
On 2015/02/04 02:35:29, michaelpg wrote:
> On 2015/02/04 02:19:15, stevenjb wrote:
> > VLOG (I know it's less convenient but we shouldn't check in ERROR for non
> > errors)
> 
> I would just remove this logging altogether.

Done. Sorry guys I must've forgotten to do 'git cl upload' one more time before
mailing the CL since I'd already removed this locally.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc:387: LOG(ERROR) <<
"url host is regular settings host";
On 2015/02/04 02:19:15, stevenjb wrote:
> And definitely not here :)

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/md_settings_ui.cc (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.cc:21: // 
html_source->SetUseJsonJSFormatV2();
On 2015/02/04 02:19:15, stevenjb wrote:
> elim or comment

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/md_settings_ui.h (right):

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:9: #include "ui/base/layout.h"
On 2015/02/04 02:19:16, stevenjb wrote:
> not needed?

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:12: class RefCountedMemory;
On 2015/02/04 02:19:16, stevenjb wrote:
> not needed?

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:14: 
On 2015/02/04 02:19:16, stevenjb wrote:
> All classes should have a comment, even trivial ones

Done.

https://codereview.chromium.org/893323002/diff/60001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:21: ui::ScaleFactor scale_factor);
On 2015/02/04 02:19:16, stevenjb wrote:
> elim

Done.

[stevenjb, 2015-02-04 18:35:20]

lgtm

[Oren Blasberg, 2015-02-04 21:33:01]

The CQ bit was checked by orenb@chromium.org

[commit-bot: I haz the power, 2015-02-04 21:33:53]

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

[commit-bot: I haz the power, 2015-02-04 21:39:57]

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

[commit-bot: I haz the power, 2015-02-04 21:39:58]

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

[James Hawkins, 2015-02-04 22:46:10]

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:6: 
nit: Remove blank line.

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:7: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared.css">
We're almost certainly not going to use this version of chrome_shared.css
(because we'll need to rewrite for MD), so let's not include that here.

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:8: <link rel="import"
href="chrome://resources/polymer/paper-button/paper-button.html">
nit: 80 cols.

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:15: <paper-button
id="manage-button" raised>Manage</paper>
Did we decide somewhere to not handle i18n for strings yet?

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/md_settings_ui.h (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:10: // The Web UI handler for
md-settings.
chrome://md-settings.

https://codereview.chromium.org/893323002/diff/80001/chrome/common/chrome_swi...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/common/chrome_swi...
chrome/common/chrome_switches.h:400: bool MdSettingsEnabled();
Why did you add this as a method?  The other methods are helpers which contain
more logic than just 'is this switch turned on'.

[stevenjb, 2015-02-04 23:00:04]

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:15: <paper-button
id="manage-button" raised>Manage</paper>
On 2015/02/04 22:46:10, James Hawkins wrote:
> Did we decide somewhere to not handle i18n for strings yet?

We did. Or at least, I suggested that perhaps we should not handle it the old
way and Michael agreed. We think we can (and should) get something better
working quickly.

(Otherwise we have to continue modifying .grd files for every string which is
kind of a pita).

https://codereview.chromium.org/893323002/diff/80001/chrome/common/chrome_swi...
File chrome/common/chrome_switches.h (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/common/chrome_swi...
chrome/common/chrome_switches.h:400: bool MdSettingsEnabled();
On 2015/02/04 22:46:10, James Hawkins wrote:
> Why did you add this as a method?  The other methods are helpers which contain
> more logic than just 'is this switch turned on'.

FWIW, I prefer helper methods since they allow changing the default in just one
place (e.g. from --enable-md-settings to --disable-md-settings).

[Oren Blasberg, 2015-02-04 23:09:36]

New patchsets have been uploaded after l-g-t-m from stevenjb@chromium.org

[Oren Blasberg, 2015-02-04 23:09:55]

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:6: 
On 2015/02/04 22:46:10, James Hawkins wrote:
> nit: Remove blank line.

Done.

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:7: <link rel="stylesheet"
href="chrome://resources/css/chrome_shared.css">
On 2015/02/04 22:46:10, James Hawkins wrote:
> We're almost certainly not going to use this version of chrome_shared.css
> (because we'll need to rewrite for MD), so let's not include that here.

Done.

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/resources...
chrome/browser/resources/md_settings/md_settings.html:8: <link rel="import"
href="chrome://resources/polymer/paper-button/paper-button.html">
On 2015/02/04 22:46:10, James Hawkins wrote:
> nit: 80 cols.

Done.

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/md_settings_ui.h (right):

https://codereview.chromium.org/893323002/diff/80001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/md_settings_ui.h:10: // The Web UI handler for
md-settings.
On 2015/02/04 22:46:10, James Hawkins wrote:
> chrome://md-settings.

Done.

[James Hawkins, 2015-02-04 23:11:04]

https://codereview.chromium.org/893323002/diff/100001/chrome/browser/resource...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/100001/chrome/browser/resource...
chrome/browser/resources/md_settings/md_settings.html:14: <paper-button
id="manage-button" raised>Manage</paper>
Is this not supposed to be </paper-button>.

[Oren Blasberg, 2015-02-04 23:17:22]

https://codereview.chromium.org/893323002/diff/100001/chrome/browser/resource...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/100001/chrome/browser/resource...
chrome/browser/resources/md_settings/md_settings.html:14: <paper-button
id="manage-button" raised>Manage</paper>
On 2015/02/04 23:11:04, James Hawkins wrote:
> Is this not supposed to be </paper-button>.

Done. (oops)

[James Hawkins, 2015-02-04 23:20:50]

LGTM with nits.

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/resource...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/resource...
chrome/browser/resources/md_settings/md_settings.html:2: <html
i18n-values="dir:textdirection;lang:language">
This i18n-values is the old system, so let's remove it.

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/md_settings_ui.h (right):

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/ui/webui...
chrome/browser/ui/webui/md_settings_ui.h:10: // The Web UI handler for
chrome://md-settings.
nit: WebUI

[Oren Blasberg, 2015-02-04 23:23:00]

New patchsets have been uploaded after l-g-t-m from jhawkins@chromium.org

[Oren Blasberg, 2015-02-04 23:23:12]

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/resource...
File chrome/browser/resources/md_settings/md_settings.html (right):

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/resource...
chrome/browser/resources/md_settings/md_settings.html:2: <html
i18n-values="dir:textdirection;lang:language">
On 2015/02/04 23:20:50, James Hawkins wrote:
> This i18n-values is the old system, so let's remove it.

Done.

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/md_settings_ui.h (right):

https://codereview.chromium.org/893323002/diff/120001/chrome/browser/ui/webui...
chrome/browser/ui/webui/md_settings_ui.h:10: // The Web UI handler for
chrome://md-settings.
On 2015/02/04 23:20:50, James Hawkins wrote:
> nit: WebUI

Done.

[Oren Blasberg, 2015-02-04 23:23:45]

The CQ bit was checked by orenb@chromium.org

[commit-bot: I haz the power, 2015-02-04 23:24:40]

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

[commit-bot: I haz the power, 2015-02-05 00:47:38]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-02-05 00:48:39]

Patchset 8 (id:??) landed as
https://crrev.com/91bc7932a4c33873515a2237dbf4ab933e16bb94
Cr-Commit-Position: refs/heads/master@{#314685}