Add new chrome://proximity-auth WebUI for debugging Smart Lock.

Add new chrome://proximity-auth WebUI for debugging Smart Lock.

This CL only hooks up the new WebUI for ChromeOS. The page itself is just placeholder for
now.

BUG=chromium:409158
TEST=manual

Committed: https://crrev.com/cc68959f56e73ef2207cc0fdf0507abcbcf11ef9
Cr-Commit-Position: refs/heads/master@{#326688}

[Tim Song, 2015-04-13 23:50:46]

Patchset #1 (id:1) has been deleted

[Tim Song, 2015-04-14 00:01:02]

Patchset #1 (id:20001) has been deleted

[Tim Song, 2015-04-14 00:01:11]

Patchset #1 (id:20001) has been deleted

[Tim Song, 2015-04-14 00:01:58]

tengs@chromium.org changed reviewers:
+ isherman@chromium.org

[Ilya Sherman, 2015-04-14 00:25:44]

Can we hook up the UI on all (desktop?) OSes, rather than just on ChromeOS?  I'd
love to be able to develop on Mac :)

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/BUILD.gn (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/BUILD.gn:19:
"//components/proximity_auth/cryptauth",
Hmm, why does the webui depend on both of these?

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/BUILD.gn:21: "//content/public/browser",
We'll need to create a DEPS file for the webui subdirectory, right?

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_ui.h (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_ui.h:19:
ProximityAuthUI(content::WebUI* web_ui);
nit: explicit

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_ui.h:19:
ProximityAuthUI(content::WebUI* web_ui);
Please document lifetime or ownership semantics for |web_ui|.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_constants.h (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_constants.h:6: #define
COMPONENTS_PROXIMITY_AUTH_WEBUI_PROXIMITY_AUTH_WEBUI_CONSTANTS_H_
nit: Do we really need to repeat |proximity_auth_webui| twice in this filename? 
Maybe we should just name this "constants.h" or "webui_constants.h" or something
briefer like that?

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:10: #include
"content/public/browser/web_ui.h"
Are any of these includes needed?

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/resources/proximity_auth.js (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/resources/proximity_auth.js:91:
document.addEventListener("DOMContentLoaded", draw);
Can we leave this file essentially empty, rather than creating this ad-hoc
implementation that's not meant to last long-term?  I'd rather not review this
code, which is fairly conceptually dense, if it's not going to last; but we also
shouldn't check in unreviewed code.

[Tim Song, 2015-04-14 06:00:12]

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/BUILD.gn (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/BUILD.gn:19:
"//components/proximity_auth/cryptauth",
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> Hmm, why does the webui depend on both of these?

I want to also add UI to make CryptAuth calls, like GetMyDevices, which is very
useful for debugging the server state.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/BUILD.gn:21: "//content/public/browser",
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> We'll need to create a DEPS file for the webui subdirectory, right?

Sorry, forgot to add the DEPS file.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_ui.h (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_ui.h:19:
ProximityAuthUI(content::WebUI* web_ui);
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> nit: explicit

Done.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_ui.h:19:
ProximityAuthUI(content::WebUI* web_ui);
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> Please document lifetime or ownership semantics for |web_ui|.

Done.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_constants.h (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_constants.h:6: #define
COMPONENTS_PROXIMITY_AUTH_WEBUI_PROXIMITY_AUTH_WEBUI_CONSTANTS_H_
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> nit: Do we really need to repeat |proximity_auth_webui| twice in this
filename? 
> Maybe we should just name this "constants.h" or "webui_constants.h" or
something
> briefer like that?

Done. Renamed to url_constants for consistency with the other url_constants
files.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/proximity_auth_webui_handler.cc (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/proximity_auth_webui_handler.cc:10: #include
"content/public/browser/web_ui.h"
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> Are any of these includes needed?

Done. Not at the moment. I'll add them back once we start adding message
handlers.

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/resources/proximity_auth.js (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/resources/proximity_auth.js:91:
document.addEventListener("DOMContentLoaded", draw);
On 2015/04/14 00:25:44, Ilya Sherman wrote:
> Can we leave this file essentially empty, rather than creating this ad-hoc
> implementation that's not meant to last long-term?  I'd rather not review this
> code, which is fairly conceptually dense, if it's not going to last; but we
also
> shouldn't check in unreviewed code.

This is a placeholder to confirm that chrome://proximity-auth indeed loads
something. As just some arbitrary sandboxed JS, I think we're fine without a
review. Plus, it looks really cool.

[Ilya Sherman, 2015-04-14 21:47:53]

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/BUILD.gn (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/BUILD.gn:19:
"//components/proximity_auth/cryptauth",
On 2015/04/14 06:00:12, Tim Song wrote:
> On 2015/04/14 00:25:44, Ilya Sherman wrote:
> > Hmm, why does the webui depend on both of these?
> 
> I want to also add UI to make CryptAuth calls, like GetMyDevices, which is
very
> useful for debugging the server state.

Let's add deps as they become relevant, rather than trying to add all the likely
candidates upfront.

https://codereview.chromium.org/1080163002/diff/60001/components/proximity_au...
File components/proximity_auth.gypi (right):

https://codereview.chromium.org/1080163002/diff/60001/components/proximity_au...
components/proximity_auth.gypi:127: 'proximity_auth/webui/url_constants.h',
nit: Please re-alphabetize.

https://codereview.chromium.org/1080163002/diff/60001/components/resources/co...
File components/resources/components_resources.grd (right):

https://codereview.chromium.org/1080163002/diff/60001/components/resources/co...
components/resources/components_resources.grd:16: <if expr="chromeos">
Why just on ChromeOS?  Is there harm to users visiting this page on other
platforms?

[Tim Song, 2015-04-14 23:24:05]

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
File components/proximity_auth/webui/BUILD.gn (right):

https://codereview.chromium.org/1080163002/diff/40001/components/proximity_au...
components/proximity_auth/webui/BUILD.gn:19:
"//components/proximity_auth/cryptauth",
On 2015/04/14 21:47:53, Ilya Sherman wrote:
> On 2015/04/14 06:00:12, Tim Song wrote:
> > On 2015/04/14 00:25:44, Ilya Sherman wrote:
> > > Hmm, why does the webui depend on both of these?
> > 
> > I want to also add UI to make CryptAuth calls, like GetMyDevices, which is
> very
> > useful for debugging the server state.
> 
> Let's add deps as they become relevant, rather than trying to add all the
likely
> candidates upfront.

Done.

https://codereview.chromium.org/1080163002/diff/60001/components/resources/co...
File components/resources/components_resources.grd (right):

https://codereview.chromium.org/1080163002/diff/60001/components/resources/co...
components/resources/components_resources.grd:16: <if expr="chromeos">
On 2015/04/14 21:47:53, Ilya Sherman wrote:
> Why just on ChromeOS?  Is there harm to users visiting this page on other
> platforms?

Done. This was left over from when this page was CrOS only.

[Ilya Sherman, 2015-04-14 23:27:56]

LGTM % nit.  Thanks.

https://codereview.chromium.org/1080163002/diff/80001/components/proximity_au...
File components/proximity_auth.gypi (right):

https://codereview.chromium.org/1080163002/diff/80001/components/proximity_au...
components/proximity_auth.gypi:127: 'proximity_auth/webui/url_constants.h',
nit: Please alphabetize.

[Tim Song, 2015-04-15 16:39:37]

tengs@chromium.org changed reviewers:
+ blundell@chromium.org, sievers@chromium.org, sky@chromium.org,
xiyuan@chromium.org

[Tim Song, 2015-04-15 16:39:38]

+sky@ for chrome/browser/ui/BUILD.gn
+xiyuan@ for chrome/browser/ui/webui
+blundell@ for components/resources/
+sievers@ for +content/public/browser DEPS

https://codereview.chromium.org/1080163002/diff/80001/components/proximity_au...
File components/proximity_auth.gypi (right):

https://codereview.chromium.org/1080163002/diff/80001/components/proximity_au...
components/proximity_auth.gypi:127: 'proximity_auth/webui/url_constants.h',
On 2015/04/14 23:27:56, Ilya Sherman wrote:
> nit: Please alphabetize.

Done.

[blundell, 2015-04-15 17:06:18]

lgtm

Could you coordinate with agable@ to ensure that the new grdp file starts
getting included in the translation process?

[Tim Song, 2015-04-15 17:08:53]

Do we really care about translations for WebUI that isn't intended to be user
facing?

[blundell, 2015-04-15 17:10:42]

On 2015/04/15 17:08:53, Tim Song wrote:
> Do we really care about translations for WebUI that isn't intended to be user
> facing?

I'm not the one who sets this kind of policy, but I would assume that the safest
policy is to have all of our grd(p) files included in the translation process.
If this feature added user-facing strings, would they not end up in the same
file?

[xiyuan, 2015-04-15 17:10:47]

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
File components/proximity_auth/webui/resources/proximity_auth.css (right):

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
components/proximity_auth/webui/resources/proximity_auth.css:6: html, body {
nit: Specify one selector per line.

https://www.chromium.org/developers/web-development-style-guide

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
File components/proximity_auth/webui/resources/proximity_auth.html (right):

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
components/proximity_auth/webui/resources/proximity_auth.html:11: <title
i18n-content="proximityAuthTitle"></title>
This probably does not work as additional work is needed to support i18n.

We would need 

  <script src="chrome://resources/js/i18n_template.js"></script>
  <script src="chrome://resources/js/jstemplate_compiled.js"></script>

at the end of html.

and define a string in grd and AddLocalizedString("proximityAuthTitle", ...)
when creating the webui data source.

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
File components/proximity_auth/webui/resources/proximity_auth.js (right):

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
components/proximity_auth/webui/resources/proximity_auth.js:88: //while
(commands.length) drawCommand();
nit: remove?

[xiyuan, 2015-04-15 17:39:37]

If this is a non end-user facing diagnosing WebUI, we seem to have some
precedance with untranslated html, 

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

net internal:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

[blundell, 2015-04-15 18:48:38]

On 2015/04/15 17:39:37, xiyuan wrote:
> If this is a non end-user facing diagnosing WebUI, we seem to have some
> precedance with untranslated html, 
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
> 
> net internal:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...

My question is about having the grdp be translated. I agree that for
non-user-facing strings it doesn't matter. My guess is that any user-facing
strings would go in that same file.

[Tim Song, 2015-04-15 19:42:07]

Hmm... we may have user facing resources in the future, so it's probably worth
it getting the grdp into the translation pipeline.

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
File components/proximity_auth/webui/resources/proximity_auth.css (right):

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
components/proximity_auth/webui/resources/proximity_auth.css:6: html, body {
On 2015/04/15 17:10:47, xiyuan wrote:
> nit: Specify one selector per line.
> 
> https://www.chromium.org/developers/web-development-style-guide

Done.

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
File components/proximity_auth/webui/resources/proximity_auth.html (right):

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
components/proximity_auth/webui/resources/proximity_auth.html:11: <title
i18n-content="proximityAuthTitle"></title>
On 2015/04/15 17:10:47, xiyuan wrote:
> This probably does not work as additional work is needed to support i18n.
> 
> We would need 
> 
>   <script src="chrome://resources/js/i18n_template.js"></script>
>   <script src="chrome://resources/js/jstemplate_compiled.js"></script>
> 
> at the end of html.
> 
> and define a string in grd and AddLocalizedString("proximityAuthTitle", ...)
> when creating the webui data source.

I removed this string for now. I'll add the localization support once I start
implementing the UI.

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
File components/proximity_auth/webui/resources/proximity_auth.js (right):

https://codereview.chromium.org/1080163002/diff/120001/components/proximity_a...
components/proximity_auth/webui/resources/proximity_auth.js:88: //while
(commands.length) drawCommand();
On 2015/04/15 17:10:47, xiyuan wrote:
> nit: remove?

Done.

[xiyuan, 2015-04-15 19:43:14]

lgtm

[sky, 2015-04-16 15:40:36]

chrome/browser/ui/BUILD.gn LGTM

[no sievers, 2015-04-23 19:58:01]

components/proximity_auth/webui/DEPS lgtm

[Tim Song, 2015-04-23 21:03:25]

The CQ bit was checked by tengs@chromium.org

[Tim Song, 2015-04-23 21:03:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
isherman@chromium.org, blundell@chromium.org
Link to the patchset: https://codereview.chromium.org/1080163002/#ps140001
(title: "fixes")

[commit-bot: I haz the power, 2015-04-23 21:03:40]

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

[commit-bot: I haz the power, 2015-04-23 23:08:23]

Committed patchset #6 (id:140001)

[commit-bot: I haz the power, 2015-04-23 23:10:04]

Patchset 6 (id:??) landed as
https://crrev.com/cc68959f56e73ef2207cc0fdf0507abcbcf11ef9
Cr-Commit-Position: refs/heads/master@{#326688}