Send the value of google_brand::GetBrand in component update checks.

Send the value of google_brand::GetBrand in component update checks.

BUG=594358
Committed: https://crrev.com/a1fafb7d1fe7ddd214c97f47bc74a97f0848b68d
Cr-Commit-Position: refs/heads/master@{#382876}

[Sorin Jianu, 2016-03-22 22:51:04]

Description was changed from

==========
Send the value of google_brand::GetBrand in component update checks.

BUG=594358
==========

to

==========
Send the value of google_brand::GetBrand in component update checks.

BUG=594358
==========

[Sorin Jianu, 2016-03-22 22:51:04]

sorin@chromium.org changed reviewers:
+ asargent@chromium.org, droger@chromium.org, waffles@chromium.org

[Sorin Jianu, 2016-03-22 22:51:12]

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

[commit-bot: I haz the power, 2016-03-22 22:51:34]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1823273002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1823273002/1

[Sorin Jianu, 2016-03-22 22:56:30]

David, Antony, I need owners approval for the paths under iOS and extensions
respectively.

Thank you!

[commit-bot: I haz the power, 2016-03-22 22:58:40]

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

[commit-bot: I haz the power, 2016-03-22 22:58:41]

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

[waffles, 2016-03-22 23:01:34]

https://codereview.chromium.org/1823273002/diff/1/components/update_client/up...
File components/update_client/update_checker.cc (right):

https://codereview.chromium.org/1823273002/diff/1/components/update_client/up...
components/update_client/update_checker.cc:56: base::StringAppendF(&app, "
brand=\"%s\"", brand.c_str());
Can we sanitize brand (to "^([a-zA-Z]{4})?$") before appending it like this?
IIUC, there isn't any sanitization in google_brand::GetBrand.

[asargent_no_longer_on_chrome, 2016-03-23 00:08:28]

lgtm

Something to think about for future work - the fact that you had to duplicate
the same code in 3 places indicates we could probably refactor things to get
more sharing, possibly with something like a ChromeUpdateClientConfigurator
superclass that has all the common stuff.

[Sorin Jianu, 2016-03-23 01:10:58]

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

[commit-bot: I haz the power, 2016-03-23 01:11:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1823273002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1823273002/20001

[Sorin Jianu, 2016-03-23 02:10:32]

Thank so much PTAL.

I am aware of the code duplication but I don't have a solution that is simple
enough. The issue here is that the brand is something that is browser-dependent.
It has to be injected from the embedder. So it is possible to create an
abstraction that injects the brand, and to a large extent, this is what we have
now. The abstraction is a function google_brand::GetBrand(). We were able to
reuse some functionality of the configurator for the code in src/components,
which is common for all embedders.

https://codereview.chromium.org/1823273002/diff/1/components/update_client/up...
File components/update_client/update_checker.cc (right):

https://codereview.chromium.org/1823273002/diff/1/components/update_client/up...
components/update_client/update_checker.cc:56: base::StringAppendF(&app, "
brand=\"%s\"", brand.c_str());
On 2016/03/22 23:01:34, waffles wrote:
> Can we sanitize brand (to "^([a-zA-Z]{4})?$") before appending it like this?
> IIUC, there isn't any sanitization in google_brand::GetBrand.

Done.

[commit-bot: I haz the power, 2016-03-23 02:16:03]

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

[commit-bot: I haz the power, 2016-03-23 02:16:04]

Dry run: This issue passed the CQ dry run.

[droger, 2016-03-23 07:22:12]

lgtm

[waffles, 2016-03-23 14:16:48]

lgtm

[Sorin Jianu, 2016-03-23 17:02:50]

The CQ bit was checked by sorin@chromium.org

[Sorin Jianu, 2016-03-23 17:02:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
asargent@chromium.org, waffles@chromium.org, droger@chromium.org
Link to the patchset: https://codereview.chromium.org/1823273002/#ps40001
(title: "minor unit test improvement.")

[commit-bot: I haz the power, 2016-03-23 17:03:03]

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

[commit-bot: I haz the power, 2016-03-23 17:54:53]

Description was changed from

==========
Send the value of google_brand::GetBrand in component update checks.

BUG=594358
==========

to

==========
Send the value of google_brand::GetBrand in component update checks.

BUG=594358
==========

[commit-bot: I haz the power, 2016-03-23 17:54:54]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-03-23 17:56:24]

Description was changed from

==========
Send the value of google_brand::GetBrand in component update checks.

BUG=594358
==========

to

==========
Send the value of google_brand::GetBrand in component update checks.

BUG=594358

Committed: https://crrev.com/a1fafb7d1fe7ddd214c97f47bc74a97f0848b68d
Cr-Commit-Position: refs/heads/master@{#382876}
==========

[commit-bot: I haz the power, 2016-03-23 17:56:25]

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