Move resources for version_ui to //components/version_ui

Move resources for version_ui to //components/version_ui

These files are moved in order to be shared on iOS.

- moved the resources to //components/version_ui/resources
- moved the strings to //components
- Merged version_info strings into version_ui strings.
- Renamed IDX_ABOUT_VERSION_* to IDX_VERSION_UI_*.
- moved the constants to //components/version_ui/version_ui_constants

TBR=jochen

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

[droger, 2015-09-14 11:25:42]

Patchset #1 (id:1) has been deleted

[droger, 2015-09-14 11:25:49]

Patchset #1 (id:20001) has been deleted

[droger, 2015-09-14 11:32:55]

Patchset #1 (id:40001) has been deleted

[droger, 2015-09-14 13:21:25]

Patchset #1 (id:60001) has been deleted

[droger, 2015-09-14 13:21:51]

Patchset #1 (id:80001) has been deleted

[droger, 2015-09-14 14:09:04]

Patchset #1 (id:100001) has been deleted

[droger, 2015-09-14 14:24:01]

Patchset #1 (id:120001) has been deleted

[droger, 2015-09-14 15:31:18]

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

[droger, 2015-09-14 15:31:19]

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
File components/version_ui/resources/about_version.html (right):

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
components/version_ui/resources/about_version.html:34: <img
src="../../../chrome/app/theme/default_100_percent/%DISTRIBUTION%/product_logo.png">
dbeam:

Even though it works as is, this dependency on a resource in //chrome looks
conceptually wrong.
I wanted to inject this image from the C++ code, but I could not find how to do
that.

An alternative would be to move this resource to //components, but that requires
also moving the corresponding Google-specific resource, I am not sure we want to
do that.

Do you have any recommendation?

[droger, 2015-09-14 15:32:33]

CC blundell, as we talked about this image issue offline.

[droger, 2015-09-14 15:37:07]

Patchset #1 (id:140001) has been deleted

[Dan Beam, 2015-09-16 01:19:19]

this conceptually seems fine, but if there are //chrome dependencies, I don't
really see how to get around moving them to //components?

also, we should really write a grd analyzer that just generates
tools/gritsettings/resource_ids perfectly (i.e. looks into each .grd, figures
out how many resource IDs it needs, etc.)

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
File components/version_ui/resources/about_version.html (right):

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
components/version_ui/resources/about_version.html:34: <img
src="../../../chrome/app/theme/default_100_percent/%DISTRIBUTION%/product_logo.png">
On 2015/09/14 15:31:19, droger wrote:
> dbeam:
> 
> Even though it works as is, this dependency on a resource in //chrome looks
> conceptually wrong.
> I wanted to inject this image from the C++ code, but I could not find how to
do
> that.
> 
> An alternative would be to move this resource to //components, but that
requires
> also moving the corresponding Google-specific resource, I am not sure we want
to
> do that.

why are you not sure we'd want to do that?

> 
> Do you have any recommendation?

injecting from C++ is less direct.  you'd probably need to base64 encode the
contents of the image, pass those contents the JS, and dynamically populate the
image with a data URI

[droger, 2015-09-16 15:07:16]

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
File components/version_ui/resources/about_version.html (right):

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
components/version_ui/resources/about_version.html:34: <img
src="../../../chrome/app/theme/default_100_percent/%DISTRIBUTION%/product_logo.png">
On 2015/09/16 01:19:19, Dan Beam wrote:
> why are you not sure we'd want to do that?

Because it involves setting up an infrastructure to have Google-specific
resources in components (which introduces complexity, and the resources will be
no longer all in the same place).

That said we did the work for the strings (we can have Google-specific strings
in components) so why not have specific resources as well.
I thought injecting it from C++ would be simpler, but if it's not possible we
can probably go with that.

[Dan Beam, 2015-09-16 17:13:56]

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
File components/version_ui/resources/about_version.html (right):

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
components/version_ui/resources/about_version.html:34: <img
src="../../../chrome/app/theme/default_100_percent/%DISTRIBUTION%/product_logo.png">
On 2015/09/16 15:07:16, droger wrote:
> On 2015/09/16 01:19:19, Dan Beam wrote:
> > why are you not sure we'd want to do that?
> 
> Because it involves setting up an infrastructure to have Google-specific
> resources in components (which introduces complexity, and the resources will
be
> no longer all in the same place).
> 
> That said we did the work for the strings (we can have Google-specific strings
> in components) so why not have specific resources as well.
> I thought injecting it from C++ would be simpler, but if it's not possible we
> can probably go with that.

it's possible, but not simpler.  it'd probably also encourage bad practices if
others start copying this.

[droger, 2015-10-13 15:58:30]

Patchset #3 (id:200001) has been deleted

[droger, 2015-10-13 15:58:37]

Patchset #3 (id:220001) has been deleted

[droger, 2015-10-13 15:58:43]

Patchset #3 (id:240001) has been deleted

[droger, 2015-10-13 16:30:17]

Please take another look.

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
File components/version_ui/resources/about_version.html (right):

https://codereview.chromium.org/1332353003/diff/180001/components/version_ui/...
components/version_ui/resources/about_version.html:34: <img
src="../../../chrome/app/theme/default_100_percent/%DISTRIBUTION%/product_logo.png">
On 2015/09/16 17:13:56, Dan Beam wrote:
> On 2015/09/16 15:07:16, droger wrote:
> > On 2015/09/16 01:19:19, Dan Beam wrote:
> > > why are you not sure we'd want to do that?
> > 
> > Because it involves setting up an infrastructure to have Google-specific
> > resources in components (which introduces complexity, and the resources will
> be
> > no longer all in the same place).
> > 
> > That said we did the work for the strings (we can have Google-specific
strings
> > in components) so why not have specific resources as well.
> > I thought injecting it from C++ would be simpler, but if it's not possible
we
> > can probably go with that.
> 
> it's possible, but not simpler.  it'd probably also encourage bad practices if
> others start copying this.

Done. The resource has been moved to components.

[Dan Beam, 2015-10-15 04:02:35]

lgtm otherwise

https://codereview.chromium.org/1332353003/diff/260001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/version_handler.cc (right):

https://codereview.chromium.org/1332353003/diff/260001/chrome/browser/ui/webu...
chrome/browser/ui/webui/version_handler.cc:39: }
nit: no more curlies for either of this conditionals

https://codereview.chromium.org/1332353003/diff/260001/components/resources/v...
File components/resources/version_ui_resources.grdp (right):

https://codereview.chromium.org/1332353003/diff/260001/components/resources/v...
components/resources/version_ui_resources.grdp:3: <include
name="IDR_VERSION_UI_CSS" file="../version_ui/resources/about_version.css"
type="BINDATA" />
this is kind of weird... why wouldn't a .grd that has .grdp's in various sub
directory live in the common ancestor?

i.e.

  components/

    can_access_version_ui_and_resources.grd
      <part file=version_ui/some.grdp>
      <part file=resources/some.grdp>

    version_ui/
      some.grdp

    resources/
      some.grdp

or whatever

https://codereview.chromium.org/1332353003/diff/260001/components/version_ui_...
File components/version_ui_strings.grdp (right):

https://codereview.chromium.org/1332353003/diff/260001/components/version_ui_...
components/version_ui_strings.grdp:2: <grit-part>
treat every tag except <if> like HTML/XML and indent \s\s for each nested level
(and \s\s\s\s\ for continuations, if that happens)

[droger, 2015-10-15 15:21:57]

Thanks for the review.

https://codereview.chromium.org/1332353003/diff/260001/components/resources/v...
File components/resources/version_ui_resources.grdp (right):

https://codereview.chromium.org/1332353003/diff/260001/components/resources/v...
components/resources/version_ui_resources.grdp:3: <include
name="IDR_VERSION_UI_CSS" file="../version_ui/resources/about_version.css"
type="BINDATA" />
On 2015/10/15 04:02:35, Dan Beam wrote:
> this is kind of weird... why wouldn't a .grd that has .grdp's in various sub
> directory live in the common ancestor?
> 
> i.e.
> 
>   components/
> 
>     can_access_version_ui_and_resources.grd
>       <part file=version_ui/some.grdp>
>       <part file=resources/some.grdp>
> 
>     version_ui/
>       some.grdp
> 
>     resources/
>       some.grdp
> 
> or whatever

I agree, but all components resources are architectured like this.
In other words, this file is consistent with the ~15 other resource files in
this directory.

I think changing this is out of scope of this CL.

[Dan Beam, 2015-10-15 18:19:40]

On 2015/10/15 15:21:57, droger wrote:
> Thanks for the review.
> 
>
https://codereview.chromium.org/1332353003/diff/260001/components/resources/v...
> File components/resources/version_ui_resources.grdp (right):
> 
>
https://codereview.chromium.org/1332353003/diff/260001/components/resources/v...
> components/resources/version_ui_resources.grdp:3: <include
> name="IDR_VERSION_UI_CSS" file="../version_ui/resources/about_version.css"
> type="BINDATA" />
> On 2015/10/15 04:02:35, Dan Beam wrote:
> > this is kind of weird... why wouldn't a .grd that has .grdp's in various sub
> > directory live in the common ancestor?
> > 
> > i.e.
> > 
> >   components/
> > 
> >     can_access_version_ui_and_resources.grd
> >       <part file=version_ui/some.grdp>
> >       <part file=resources/some.grdp>
> > 
> >     version_ui/
> >       some.grdp
> > 
> >     resources/
> >       some.grdp
> > 
> > or whatever
> 
> I agree, but all components resources are architectured like this.
> In other words, this file is consistent with the ~15 other resource files in
> this directory.
> 
> I think changing this is out of scope of this CL.

that's fine, but we should probably still do it

[droger, 2015-10-16 08:49:51]

droger@chromium.org changed reviewers:
+ jochen@chromium.org

[droger, 2015-10-16 08:49:52]

Thanks for the review.
I filed http://crbug.com/543996 for the directory structure.

TBR=jochen for
resettable_settings_snapshot.cc (trivial changes)
chrome/browser/ui/DEPS
chrome/browser/ui/BUILD.gn
components/version_info/version_info.cc (trivial changes)

[droger, 2015-10-16 08:50:10]

The CQ bit was checked by droger@chromium.org

[droger, 2015-10-16 08:50:10]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org
Link to the patchset: https://codereview.chromium.org/1332353003/#ps280001
(title: "Review comments")

[commit-bot: I haz the power, 2015-10-16 08:50:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1332353003/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1332353003/280001

[commit-bot: I haz the power, 2015-10-16 08:58:11]

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

[commit-bot: I haz the power, 2015-10-16 08:58:12]

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

[droger, 2015-10-16 09:00:37]

The CQ bit was checked by droger@chromium.org

[commit-bot: I haz the power, 2015-10-16 09:02:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1332353003/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1332353003/280001

[commit-bot: I haz the power, 2015-10-16 09:07:09]

Committed patchset #4 (id:280001)

[commit-bot: I haz the power, 2015-10-16 09:07:48]

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