Chromium Code Reviews
[email protected] (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(490)

Issue 1854093002: Enable InstanceID field trial by default (Closed)

Created:
4 years, 8 months ago by johnme
Modified:
4 years, 7 months ago
CC:
chromium-reviews, Peter Beverloo, chromium-apps-reviews_chromium.org, johnme+watch_chromium.org, zea+watch_chromium.org, extensions-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@iid3test
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Enable InstanceID field trial by default This makes it easier to write tests, since each test doesn't have to enable the field trial (which is especially hard from Java). Part of a series of patches: 1. https://codereview.chromium.org/1832833002 adds InstanceIDWithSubtype 2. https://codereview.chromium.org/1830983002 adds JNI bindings 3. https://codereview.chromium.org/1829023002 adds fake and test 4. https://codereview.chromium.org/1899753002 fixes strict mode violations 5. this patch 6. https://codereview.chromium.org/1923953002 adds crypto integration 7. https://codereview.chromium.org/1851423003 switches Push to InstanceIDs BUG=589461, 477084 Committed: https://crrev.com/384d15da99c3e9460ee87016aeb6f3387f20dc98 Cr-Commit-Position: refs/heads/master@{#390339}

Patch Set 1 #

Patch Set 2 : Rebase #

Total comments: 2

Patch Set 3 : Address review comment #

Total comments: 4

Patch Set 4 : Address asvitkine's review comments #

Patch Set 5 : Rebase #

Patch Set 6 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -18 lines) Patch
M chrome/browser/extensions/api/instance_id/instance_id_apitest.cc View 1 2 3 4 3 chunks +0 lines, -10 lines 0 comments Download
M components/gcm_driver/gcm_driver_desktop_unittest.cc View 1 2 3 4 5 4 chunks +0 lines, -6 lines 0 comments Download
M components/gcm_driver/instance_id/instance_id_driver.cc View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 22 (11 generated)
johnme
(See also discarded previous patches https://codereview.chromium.org/1736433002 and https://codereview.chromium.org/1731913002)
4 years, 8 months ago (2016-04-05 19:15:06 UTC) #3
johnme
4 years, 8 months ago (2016-04-05 19:15:15 UTC) #5
jianli
lgtm https://codereview.chromium.org/1854093002/diff/20001/chrome/browser/extensions/api/instance_id/instance_id_apitest.cc File chrome/browser/extensions/api/instance_id/instance_id_apitest.cc (right): https://codereview.chromium.org/1854093002/diff/20001/chrome/browser/extensions/api/instance_id/instance_id_apitest.cc#newcode7 chrome/browser/extensions/api/instance_id/instance_id_apitest.cc:7: #include "base/base_switches.h" nit: remove this
4 years, 8 months ago (2016-04-08 00:20:43 UTC) #6
johnme
asvitkine: please review testing/variations/ - thanks! https://codereview.chromium.org/1854093002/diff/20001/chrome/browser/extensions/api/instance_id/instance_id_apitest.cc File chrome/browser/extensions/api/instance_id/instance_id_apitest.cc (right): https://codereview.chromium.org/1854093002/diff/20001/chrome/browser/extensions/api/instance_id/instance_id_apitest.cc#newcode7 chrome/browser/extensions/api/instance_id/instance_id_apitest.cc:7: #include "base/base_switches.h" On ...
4 years, 8 months ago (2016-04-08 13:19:43 UTC) #8
Alexei Svitkine (slow)
https://codereview.chromium.org/1854093002/diff/40001/components/gcm_driver/instance_id/instance_id_driver.cc File components/gcm_driver/instance_id/instance_id_driver.cc (right): https://codereview.chromium.org/1854093002/diff/40001/components/gcm_driver/instance_id/instance_id_driver.cc#newcode23 components/gcm_driver/instance_id/instance_id_driver.cc:23: return group_name != kInstanceIDFieldTrialDisabledGroupName; Our best practices recommend using ...
4 years, 8 months ago (2016-04-08 15:11:03 UTC) #9
johnme
Addressed asvitkine's review comments - thanks! https://codereview.chromium.org/1854093002/diff/40001/components/gcm_driver/instance_id/instance_id_driver.cc File components/gcm_driver/instance_id/instance_id_driver.cc (right): https://codereview.chromium.org/1854093002/diff/40001/components/gcm_driver/instance_id/instance_id_driver.cc#newcode23 components/gcm_driver/instance_id/instance_id_driver.cc:23: return group_name != ...
4 years, 8 months ago (2016-04-08 15:28:10 UTC) #10
Peter Beverloo
lgtm
4 years, 8 months ago (2016-04-11 14:19:17 UTC) #12
Alexei Svitkine (slow)
lgtm
4 years, 8 months ago (2016-04-11 14:53:07 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1854093002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1854093002/100001
4 years, 7 months ago (2016-04-28 09:49:45 UTC) #18
commit-bot: I haz the power
Committed patchset #6 (id:100001)
4 years, 7 months ago (2016-04-28 10:32:22 UTC) #20
commit-bot: I haz the power
4 years, 7 months ago (2016-04-30 17:17:18 UTC) #21
Message was sent while issue was closed.
Patchset 6 (id:??) landed as
https://crrev.com/384d15da99c3e9460ee87016aeb6f3387f20dc98
Cr-Commit-Position: refs/heads/master@{#390339}

Powered by Google App Engine
This is Rietveld 408576698