Clean Read/SetExperimentLabels to remove check for Chrome build.

Clean Read/SetExperimentLabels to remove check for Chrome build.

BUG=729181
Review-Url: https://codereview.chromium.org/2919963002
Cr-Commit-Position: refs/heads/master@{#476846}
Committed: https://chromium.googlesource.com/chromium/src/+/9bb8c3f9afda43f4da242c0bb4bd5f500fe160bb

[nikunjb, 2017-06-02 05:08:58]

nikunjb@google.com changed reviewers:
+ grt@chromium.org, isherman@chromium.org, nikunjb@google.com,
skare@chromium.org

[nikunjb, 2017-06-02 05:08:59]

Separate out check for google chrome build from helper functions
Read/SetExperimentLabels in google_update_settings.cc

[grt (UTC plus 2), 2017-06-02 08:58:44]

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

[commit-bot: I haz the power, 2017-06-02 08:58:55]

Dry run: CQ is trying da patch.

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

[grt (UTC plus 2), 2017-06-02 09:00:39]

thanks. this will increase test coverage on Chromium builds so that we can spot
regressions on the Chromium waterfall rather than waiting for the continuous
waterfall. lgtm.

[commit-bot: I haz the power, 2017-06-02 09:34:30]

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

[commit-bot: I haz the power, 2017-06-02 09:34:31]

Dry run: Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[grt (UTC plus 2), 2017-06-02 13:29:28]

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/chrome_variations_service_client.cc
(right):

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/chrome_variations_service_client.cc:20:
#include "chrome/installer/util/install_modes.h"
installer/util -> install_static
and move into proper sorted position

[Alexei Svitkine (slow), 2017-06-02 15:11:40]

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

[Alexei Svitkine (slow), 2017-06-02 15:11:41]

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/chrome_variations_service_client.cc
(right):

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/chrome_variations_service_client.cc:53: //
labels (M57-M58 timeframe).
We have a TODO here to remove this code entirely. Instead of making further
changes, can we simply delete this code?

(The context is we used to integrate with Google Update to plumb some
experiments through to it. When we removed this support last year, we left this
code in to clean up existing registry keys. By this point, they've probably all
been cleaned up and we could just delete this code.)

[grt (UTC plus 2), 2017-06-02 15:16:52]

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/chrome_variations_service_client.cc
(right):

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/chrome_variations_service_client.cc:53: //
labels (M57-M58 timeframe).
On 2017/06/02 15:11:41, Alexei Svitkine (slow) wrote:
> We have a TODO here to remove this code entirely. Instead of making further
> changes, can we simply delete this code?
> 
> (The context is we used to integrate with Google Update to plumb some
> experiments through to it. When we removed this support last year, we left
this
> code in to clean up existing registry keys. By this point, they've probably
all
> been cleaned up and we could just delete this code.)

SGTM to delete the code in metrics. We're keeping the code in installer/util for
use in the upcoming retention experiment.

[nikunjb, 2017-06-02 20:52:23]

On 2017/06/02 15:16:52, grt (no reviews June 5) wrote:
>
https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
> File chrome/browser/metrics/variations/chrome_variations_service_client.cc
> (right):
> 
>
https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
> chrome/browser/metrics/variations/chrome_variations_service_client.cc:53: //
> labels (M57-M58 timeframe).
> On 2017/06/02 15:11:41, Alexei Svitkine (slow) wrote:
> > We have a TODO here to remove this code entirely. Instead of making further
> > changes, can we simply delete this code?
> > 
> > (The context is we used to integrate with Google Update to plumb some
> > experiments through to it. When we removed this support last year, we left
> this
> > code in to clean up existing registry keys. By this point, they've probably
> all
> > been cleaned up and we could just delete this code.)
> 
> SGTM to delete the code in metrics. We're keeping the code in installer/util
for
> use in the upcoming retention experiment.

Done. Deleted code for cleaning experiment labels on variations server startup
(Just wondering, I think cleaning registry can also be done directly from
installer on update)

[Alexei Svitkine (slow), 2017-06-02 20:56:57]

lgtm % comment

Please associate this change with a crbug

https://codereview.chromium.org/2919963002/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/variations/chrome_variations_service_client.cc
(right):

https://codereview.chromium.org/2919963002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/variations/chrome_variations_service_client.cc:18:
#include "base/threading/thread_restrictions.h"
Can some of these includes be removed now that you're removing corresponding
code?

[nikunjb, 2017-06-02 21:14:40]

The CQ bit was checked by nikunjb@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 21:15:08]

Dry run: CQ is trying da patch.

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

[nikunjb, 2017-06-02 21:31:43]

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
File chrome/browser/metrics/variations/chrome_variations_service_client.cc
(right):

https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
chrome/browser/metrics/variations/chrome_variations_service_client.cc:20:
#include "chrome/installer/util/install_modes.h"
On 2017/06/02 13:29:28, grt (no reviews June 5) wrote:
> installer/util -> install_static
> and move into proper sorted position

Acknowledged. (No longer needed)

https://codereview.chromium.org/2919963002/diff/20001/chrome/browser/metrics/...
File chrome/browser/metrics/variations/chrome_variations_service_client.cc
(right):

https://codereview.chromium.org/2919963002/diff/20001/chrome/browser/metrics/...
chrome/browser/metrics/variations/chrome_variations_service_client.cc:18:
#include "base/threading/thread_restrictions.h"
On 2017/06/02 20:56:57, Alexei Svitkine (slow) wrote:
> Can some of these includes be removed now that you're removing corresponding
> code?

Done. Removed all includes here (none were needed anymore)

[nikunjb, 2017-06-02 21:43:38]

On 2017/06/02 21:31:43, nikunjb wrote:
>
https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
> File chrome/browser/metrics/variations/chrome_variations_service_client.cc
> (right):
> 
>
https://codereview.chromium.org/2919963002/diff/1/chrome/browser/metrics/vari...
> chrome/browser/metrics/variations/chrome_variations_service_client.cc:20:
> #include "chrome/installer/util/install_modes.h"
> On 2017/06/02 13:29:28, grt (no reviews June 5) wrote:
> > installer/util -> install_static
> > and move into proper sorted position
> 
> Acknowledged. (No longer needed)
> 
>
https://codereview.chromium.org/2919963002/diff/20001/chrome/browser/metrics/...
> File chrome/browser/metrics/variations/chrome_variations_service_client.cc
> (right):
> 
>
https://codereview.chromium.org/2919963002/diff/20001/chrome/browser/metrics/...
> chrome/browser/metrics/variations/chrome_variations_service_client.cc:18:
> #include "base/threading/thread_restrictions.h"
> On 2017/06/02 20:56:57, Alexei Svitkine (slow) wrote:
> > Can some of these includes be removed now that you're removing corresponding
> > code?
> 
> Done. Removed all includes here (none were needed anymore)

Attaching bug 729181
(https://bugs.chromium.org/p/chromium/issues/detail?id=729181) to the change.

[nikunjb, 2017-06-02 21:44:25]

Description was changed from

==========
Clean Read/SetExperimentLabels to remove check for Chrome build.

BUG=
==========

to

==========
Clean Read/SetExperimentLabels to remove check for Chrome build.

BUG=729181
==========

[nikunjb, 2017-06-02 21:46:22]

The CQ bit was checked by nikunjb@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-02 21:46:57]

Dry run: CQ is trying da patch.

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

[Ilya Sherman, 2017-06-02 23:11:19]

I think you no longer need my review, but the metrics changes lgtm -- thanks!

[nikunjb, 2017-06-02 23:47:58]

The CQ bit was unchecked by nikunjb@google.com

[nikunjb, 2017-06-02 23:47:58]

The CQ bit was checked by nikunjb@google.com

[nikunjb, 2017-06-02 23:47:59]

The patchset sent to the CQ was uploaded after l-g-t-m from grt@chromium.org,
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2919963002/#ps40001
(title: "Fix includes in chrome_variations_service_client.cc")

[commit-bot: I haz the power, 2017-06-02 23:48:08]

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, 2017-06-03 01:20:22]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496447278933730,
"parent_rev": "0b479f2338e67a4d5ad7922ab9760b7b7137916b", "commit_rev":
"9bb8c3f9afda43f4da242c0bb4bd5f500fe160bb"}

[commit-bot: I haz the power, 2017-06-03 01:20:34]

Description was changed from

==========
Clean Read/SetExperimentLabels to remove check for Chrome build.

BUG=729181
==========

to

==========
Clean Read/SetExperimentLabels to remove check for Chrome build.

BUG=729181

Review-Url: https://codereview.chromium.org/2919963002
Cr-Commit-Position: refs/heads/master@{#476846}
Committed:
https://chromium.googlesource.com/chromium/src/+/9bb8c3f9afda43f4da242c0bb4bd...
==========

[commit-bot: I haz the power, 2017-06-03 01:20:36]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/9bb8c3f9afda43f4da242c0bb4bd...