Componentize script to generate UI string overrides mapping.

Componentize script to generate UI string overrides mapping.

Change the API of the script to generate both the header and source file
and to take a list of header files as input (to allow overridding strings
from components/components_strings.grd).

The script is now generating a method CreateUIStringOverrider() and hides
the arrays in the implementation file and allow customization of the
namespace in which the function and code is generated.

Introduce a new target "chrome_ui_string_overrider_factory" in the gyp
build to mimic the same target in the gn build (compiles the source file
generated by the script).

Rename the script from to generate_ui_string_overrider.py to reflect the
new role of the script and move it to components/variations/service.

Introduce .gni file to help using the script by different embedders (don't
introduce .gypi files as it is much harder to share code for gyp).

Componentize ui_string_overrider_unittest.cc.

BUG=534257
Committed: https://crrev.com/a213cebe6af1a80367336908d37f30ab6542940d
Cr-Commit-Position: refs/heads/master@{#352308}

[sdefresne, 2015-09-28 16:59:14]

Patchset #1 (id:1) has been deleted

[sdefresne, 2015-09-28 17:00:57]

[email protected] changed reviewers:
+ [email protected]

[sdefresne, 2015-09-28 17:00:58]

Please take a look.

Note that the unit tests for generate_ui_string_overrider.py (formerly
generate_resources_map.py) still pass but are not run on the bots. Here's the
test output on my machine FTR:

$ components/variations/service/generate_ui_string_overrider_unittest.py
....
----------------------------------------------------------------------
Ran 4 tests in 0.000s

OK

[sdefresne, 2015-09-28 17:04:32]

Note, the generated files looks like this:

$ cat
out/Debug/gen/chrome/browser/metrics/variations/ui_string_overrider_factory.h
// This file was generated by generate_ui_string_overrider.py. Do not edit.

#ifndef CHROME_BROWSER_METRICS_VARIATIONS_UI_STRING_OVERRIDER_FACTORY_H_
#define CHROME_BROWSER_METRICS_VARIATIONS_UI_STRING_OVERRIDER_FACTORY_H_

#include "components/variations/service/ui_string_overrider.h"

namespace chrome_variations {

// Returns an initialized UIStringOverrider.
variations::UIStringOverrider CreateUIStringOverrider();

}  // namespace chrome_variations

#endif  // CHROME_BROWSER_METRICS_VARIATIONS_UI_STRING_OVERRIDER_FACTORY_H_


$ cat
out/Debug/gen/chrome/browser/metrics/variations/ui_string_overrider_factory.cc
// This file was generated by generate_ui_string_overrider.py. Do not edit.

#include "chrome/browser/metrics/variations/ui_string_overrider_factory.h"

namespace chrome_variations {

namespace {

const size_t kNumResources = 3981;

const uint32_t kResourceHashes[] = {
    983865U,  // IDS_DOWNLOAD_REMOVE_CONFIRM_TITLE
    ...
};

const int kResourceIndices[] = {
    15194,  // IDS_DOWNLOAD_REMOVE_CONFIRM_TITLE
    ...
};

}  // namespace

variations::UIStringOverrider CreateUIStringOverrider() {
  return variations::UIStringOverrider(
      kResourceHashes, kResourceIndices, kNumResources);
}

}  // namespace chrome_variations

[Alexei Svitkine (slow), 2015-09-28 17:29:44]

[email protected] changed reviewers:
+ [email protected]

[Alexei Svitkine (slow), 2015-09-28 17:29:44]

+jwd, could you help review?

[jwd, 2015-09-29 15:02:54]

https://codereview.chromium.org/1374773002/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/variations/generated_resources_map.h (right):

https://codereview.chromium.org/1374773002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/variations/generated_resources_map.h:1: // Copyright 2014
The Chromium Authors. All rights reserved.
Is this file still needed?

https://codereview.chromium.org/1374773002/diff/20001/components/variations/s...
File components/variations/service/generate_ui_string_overrider.py (right):

https://codereview.chromium.org/1374773002/diff/20001/components/variations/s...
components/variations/service/generate_ui_string_overrider.py:182: "%s") % (
Can you use dictionary based string interpolation here. Easier to follow the
format with so many parameters.

https://codereview.chromium.org/1374773002/diff/20001/components/variations/s...
components/variations/service/generate_ui_string_overrider.py:221: "#endif  //
%s\n"
optional nit: use dictionary string interpolation instead of tuple.

[sdefresne, 2015-09-30 09:38:43]

Please take another look.

https://codereview.chromium.org/1374773002/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/variations/generated_resources_map.h (right):

https://codereview.chromium.org/1374773002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/variations/generated_resources_map.h:1: // Copyright 2014
The Chromium Authors. All rights reserved.
On 2015/09/29 at 15:02:54, Jesse Doherty wrote:
> Is this file still needed?

No. Removed. Thank you.

https://codereview.chromium.org/1374773002/diff/20001/components/variations/s...
File components/variations/service/generate_ui_string_overrider.py (right):

https://codereview.chromium.org/1374773002/diff/20001/components/variations/s...
components/variations/service/generate_ui_string_overrider.py:182: "%s") % (
On 2015/09/29 at 15:02:54, Jesse Doherty wrote:
> Can you use dictionary based string interpolation here. Easier to follow the
format with so many parameters.

Done.

https://codereview.chromium.org/1374773002/diff/20001/components/variations/s...
components/variations/service/generate_ui_string_overrider.py:221: "#endif  //
%s\n"
On 2015/09/29 at 15:02:54, Jesse Doherty wrote:
> optional nit: use dictionary string interpolation instead of tuple.

Done.

[sdefresne, 2015-10-01 09:19:00]

jwd: ping?

[sdefresne, 2015-10-01 10:00:56]

On 2015/10/01 at 09:19:00, sdefresne wrote:
> jwd: ping?

BTW, failures of win bots are unrelated and due to infra issues.

[jwd, 2015-10-01 21:50:40]

lgtm

[sdefresne, 2015-10-02 09:55:23]

[email protected] changed reviewers:
+ [email protected]

[sdefresne, 2015-10-02 09:55:24]

jwd: thank you for the review

asvitkine: can you do OWNERS review (jwd review is not enough according to "git
cl owners") for
  chrome/browser/metrics/metrics_services_manager.cc
  chrome/browser/metrics/variations/BUILD.gn
  chrome/browser/metrics/variations/generate_resources_map.py
  chrome/browser/metrics/variations/generate_resources_map_unittest.py
  chrome/browser/metrics/variations/generated_resources_map.h
  chrome/browser/metrics/variations/ui_string_overrider_unittest.cc

jhawkins: can you do OWNERS review for
  chrome/app/BUILD.gn
  chrome/browser/BUILD.gn
  chrome/chrome_resources.gyp

[James Hawkins, 2015-10-02 15:07:14]

lgtm

[Alexei Svitkine (slow), 2015-10-02 16:35:16]

lgtm

[sdefresne, 2015-10-05 08:31:49]

The CQ bit was checked by [email protected]

[commit-bot: I haz the power, 2015-10-05 08:31:59]

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

[commit-bot: I haz the power, 2015-10-05 09:23:48]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-10-05 09:24:45]

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