Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2828143003:
NoStatePrefetch: update the fieldtrial_testing_config (Closed)
DescriptionNoStatePrefetch: update the fieldtrial_testing_config
Update the fieldtrial_testing_config.json with the params that match the
Finch config for 95% population of Stable at M58 rollout.
After we get samples from the nostate-prefetch groups, will update with
intended configuration for nostate-prefetch rollout.
BUG=678332
Review-Url: https://codereview.chromium.org/2828143003
Cr-Commit-Position: refs/heads/master@{#466654}
Committed: https://chromium.googlesource.com/chromium/src/+/3b0deba895e763195d439c7fcd4d5f947509904d
Patch Set 1 #Patch Set 2 : SetOmniboxMode in activity_log_browsertest #Patch Set 3 : force prerendering in customtabs tests #
Total comments: 2
Messages
Total messages: 33 (20 generated)
The CQ bit was checked by pasko@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by pasko@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== NoStatePrefetch: update the fieldtrial_testing_config Update the fieldtrial_testing_config.json with the params that match the Finch config for 95% population of Stable at M58 rollout. After we get samples from the nostate-prefetch groups, will update with intended configuration for nostate-prefetch rollout. BUG=678332 ========== to ========== NoStatePrefetch: update the fieldtrial_testing_config Update the fieldtrial_testing_config.json with the params that match the Finch config for 95% population of Stable at M58 rollout. After we get samples from the nostate-prefetch groups, will update with intended configuration for nostate-prefetch rollout. BUG=678332 ==========
pasko@chromium.org changed reviewers: + droger@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
LG but it seems some tests are broken.
On 2017/04/21 08:57:35, droger wrote: > LG but it seems some tests are broken. yep, apparently customtabs tests need prerendering enabled. Easiest would probably be to force-enable prerendering there, will look at it today.
On 2017/04/21 12:25:51, pasko wrote: > On 2017/04/21 08:57:35, droger wrote: > > LG but it seems some tests are broken. > > yep, apparently customtabs tests need prerendering enabled. Easiest would > probably be to force-enable prerendering there, will look at it today. Note sure if this can be useful, but I had a similar issue last time I changed the Finch config, and changed the tests to force prerendering enabled: https://codereview.chromium.org/2693983002/ Maybe the problem is related to this or can be solved in a similar way.
The CQ bit was checked by pasko@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/04/21 12:55:16, droger wrote: > On 2017/04/21 12:25:51, pasko wrote: > > On 2017/04/21 08:57:35, droger wrote: > > > LG but it seems some tests are broken. > > > > yep, apparently customtabs tests need prerendering enabled. Easiest would > > probably be to force-enable prerendering there, will look at it today. > > Note sure if this can be useful, but I had a similar issue last time I changed > the Finch config, and changed the tests to force prerendering enabled: > https://codereview.chromium.org/2693983002/ > > Maybe the problem is related to this or can be solved in a similar way. Thanks for the pointer, it helps! I forced prerendering the exact same way in the three failing tests and they passed locally for me. Let's see what the dryrun says.
pasko@chromium.org changed reviewers: + asargent@chromium.org, asvitkine@chromium.org
asvitkine: Please review changes in testing/variations/fieldtrial_testing_config.json asargent: Please review changes in chrome/browser/extensions/activity_log
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
asargent@chromium.org changed reviewers: + rdevlin.cronin@chromium.org
I'm no longer working on chrome, redirecting to Devlin.
asargent@chromium.org changed reviewers: - asargent@chromium.org
lgtm https://codereview.chromium.org/2828143003/diff/40001/chrome/android/javatest... File chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java (right): https://codereview.chromium.org/2828143003/diff/40001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java:158: CustomTabsConnection.getInstance((Application) mAppContext).setForcePrerender(true); Do we need to set it back to false at the end, to avoid polluting other tests?
thank you for review, David https://codereview.chromium.org/2828143003/diff/40001/chrome/android/javatest... File chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java (right): https://codereview.chromium.org/2828143003/diff/40001/chrome/android/javatest... chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabsConnectionTest.java:158: CustomTabsConnection.getInstance((Application) mAppContext).setForcePrerender(true); On 2017/04/24 11:02:12, droger wrote: > Do we need to set it back to false at the end, to avoid polluting other tests? the forcing of prerender is a per-session (=connection) attribute, and tearDown() calls into CustomTabsTestUtils.cleanupSessions(), should be enough to avoid polluting ..
extensions lgtm
The CQ bit was checked by pasko@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493043804319650,
"parent_rev": "8083bc5ed7a1989652884bec28bf775dd198b447", "commit_rev":
"3b0deba895e763195d439c7fcd4d5f947509904d"}
Message was sent while issue was closed.
Description was changed from ========== NoStatePrefetch: update the fieldtrial_testing_config Update the fieldtrial_testing_config.json with the params that match the Finch config for 95% population of Stable at M58 rollout. After we get samples from the nostate-prefetch groups, will update with intended configuration for nostate-prefetch rollout. BUG=678332 ========== to ========== NoStatePrefetch: update the fieldtrial_testing_config Update the fieldtrial_testing_config.json with the params that match the Finch config for 95% population of Stable at M58 rollout. After we get samples from the nostate-prefetch groups, will update with intended configuration for nostate-prefetch rollout. BUG=678332 Review-Url: https://codereview.chromium.org/2828143003 Cr-Commit-Position: refs/heads/master@{#466654} Committed: https://chromium.googlesource.com/chromium/src/+/3b0deba895e763195d439c7fcd4d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/3b0deba895e763195d439c7fcd4d... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
