Restore the chrome watcher pending relocation

Restore the chrome watcher pending relocation

BUG=643176
Committed: https://crrev.com/3fbf2397585ee009dd5c1b5e5f32254e5a5e360c
Cr-Commit-Position: refs/heads/master@{#415994}

[manzagop (departed), 2016-09-01 15:16:32]

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

[commit-bot: I haz the power, 2016-09-01 15:16:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[manzagop (departed), 2016-09-01 15:21:29]

manzagop@chromium.org changed reviewers:
+ scottmg@chromium.org, siggi@chromium.org

[manzagop (departed), 2016-09-01 15:21:30]

This is the short term restoration of the watcher. Please have a look!

[Sigurður Ásgeirsson, 2016-09-01 15:30:35]

siggi@chromium.org changed reviewers:
+ asvitkine@google.com

[Sigurður Ásgeirsson, 2016-09-01 15:30:36]

Adding @asvitkine to regarding consent thingy.

https://codereview.chromium.org/2306613003/diff/1/chrome/app/main_dll_loader_...
File chrome/app/main_dll_loader_win.cc (right):

https://codereview.chromium.org/2306613003/diff/1/chrome/app/main_dll_loader_...
chrome/app/main_dll_loader_win.cc:209: static GetUploadsEnabledFunction
get_uploads_enabled = nullptr;
You may want to check this with chrome-metrics folks, but I think gating
functionality on consent is strongly discouraged.

[Alexei Svitkine (slow), 2016-09-01 15:40:25]

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

[Alexei Svitkine (slow), 2016-09-01 15:40:26]

https://codereview.chromium.org/2306613003/diff/1/chrome/app/main_dll_loader_...
File chrome/app/main_dll_loader_win.cc (right):

https://codereview.chromium.org/2306613003/diff/1/chrome/app/main_dll_loader_...
chrome/app/main_dll_loader_win.cc:209: static GetUploadsEnabledFunction
get_uploads_enabled = nullptr;
On 2016/09/01 15:30:36, Sigurður Ásgeirsson wrote:
> You may want to check this with chrome-metrics folks, but I think gating
> functionality on consent is strongly discouraged.

Agreed. If you're just doing this to "save extra work since metrics won't be
reported to anywhere", please don't. We don't want consented users to have lower
performance than non consented users, since this discourages users from giving
consent.

Happy to discuss in more detail if the case is different from the above.

[manzagop (departed), 2016-09-01 15:55:46]

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

[commit-bot: I haz the power, 2016-09-01 15:56:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[manzagop (departed), 2016-09-01 16:02:49]

manzagop@chromium.org changed reviewers:
+ grt@chromium.org

[manzagop (departed), 2016-09-01 16:02:49]

Thanks. Please have another look!

grt@: could you have a look for OWNERS? Thanks.

https://codereview.chromium.org/2306613003/diff/1/chrome/app/main_dll_loader_...
File chrome/app/main_dll_loader_win.cc (right):

https://codereview.chromium.org/2306613003/diff/1/chrome/app/main_dll_loader_...
chrome/app/main_dll_loader_win.cc:209: static GetUploadsEnabledFunction
get_uploads_enabled = nullptr;
On 2016/09/01 15:40:26, Alexei Svitkine (slow) wrote:
> On 2016/09/01 15:30:36, Sigurður Ásgeirsson wrote:
> > You may want to check this with chrome-metrics folks, but I think gating
> > functionality on consent is strongly discouraged.
> 
> Agreed. If you're just doing this to "save extra work since metrics won't be
> reported to anywhere", please don't. We don't want consented users to have
lower
> performance than non consented users, since this discourages users from giving
> consent.
> 
> Happy to discuss in more detail if the case is different from the above.

Done.

[Sigurður Ásgeirsson, 2016-09-01 16:34:32]

lgtm

[commit-bot: I haz the power, 2016-09-01 17:27:06]

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

[commit-bot: I haz the power, 2016-09-01 17:27:09]

Dry run: This issue passed the CQ dry run.

[scottmg, 2016-09-01 17:47:26]

Watcher just uses regular UMA, etc. so the consent is handled there, right?

LGTM if that's correct.

[manzagop (departed), 2016-09-01 17:51:34]

On 2016/09/01 17:47:26, scottmg wrote:
> Watcher just uses regular UMA, etc. so the consent is handled there, right?

Yes, that's correct:
https://cs.chromium.org/chromium/src/components/browser_watcher/watcher_metri...

> LGTM if that's correct.

[manzagop (departed), 2016-09-01 17:53:34]

On 2016/09/01 17:51:34, manzagop wrote:
> On 2016/09/01 17:47:26, scottmg wrote:
> > Watcher just uses regular UMA, etc. so the consent is handled there, right?
> 
> Yes, that's correct:
>
https://cs.chromium.org/chromium/src/components/browser_watcher/watcher_metri...
> 
> > LGTM if that's correct.

Just noticed Scott's OWNER of this specific file, so I'll go ahead with
submission.

Thanks everyone!

[manzagop (departed), 2016-09-01 17:53:52]

The CQ bit was checked by manzagop@chromium.org

[commit-bot: I haz the power, 2016-09-01 17:54:21]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-01 17:58:51]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-01 18:01:51]

Description was changed from

==========
Restore the chrome watcher pending relocation

BUG=643176
==========

to

==========
Restore the chrome watcher pending relocation

BUG=643176

Committed: https://crrev.com/3fbf2397585ee009dd5c1b5e5f32254e5a5e360c
Cr-Commit-Position: refs/heads/master@{#415994}
==========

[commit-bot: I haz the power, 2016-09-01 18:01:52]

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