New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 727044 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Use ScopedFeatureList instead of change CommandLine arguments directly in BrowserTest SetUp

Project Member Reported by chaopeng@chromium.org, May 27 2017

Issue description

Currently we have 2 ways to modify FeatureList in BrowserTest. 

1. Use ScopedFeatureList
2. Change CommandLine args

We want to unify the code path so only ScopedFeatureList can use for this purpose.

1. We need a patch to modify all calls to use ScopedFeatureList.
2. Clear features from command line in https://cs.chromium.org/chromium/src/base/test/test_suite.cc?rcl=d71bc7b31de04a31db7c360da95c5d1ff53b6784&l=350 Add DCHECK at https://cs.chromium.org/chromium/src/content/public/test/browser_test_base.cc?rcl=d71bc7b31de04a31db7c360da95c5d1ff53b6784&l=238 prevent teams try to modify FeatureList by changing CommandLine args.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Jun 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9fb15b84bfb031527e6875b0c73c5e8ee3a76401

commit 9fb15b84bfb031527e6875b0c73c5e8ee3a76401
Author: isherman <isherman@chromium.org>
Date: Sat Jun 10 01:30:34 2017

[Cleanup] Transition NetworkTime browser tests to use a ScopedFeatureList.

BUG= 727044 
TEST=browser_tests --gtest_filter=NetworkTimePolicyTest.*

Review-Url: https://codereview.chromium.org/2929713002
Cr-Commit-Position: refs/heads/master@{#478493}

[modify] https://crrev.com/9fb15b84bfb031527e6875b0c73c5e8ee3a76401/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/9fb15b84bfb031527e6875b0c73c5e8ee3a76401/chrome/browser/ssl/ssl_browser_tests.cc

Comment 3 by bokan@chromium.org, Jun 13 2017

Components: Internals>Metrics
Labels: OS-All
Status: Assigned (was: Untriaged)

Comment 4 by holte@chromium.org, Sep 1 2017

Components: -Internals>Metrics Internals>Metrics>Variations
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01e3b250a05ae29fbe847300d4d5a075b17b73af

commit 01e3b250a05ae29fbe847300d4d5a075b17b73af
Author: chaopeng <chaopeng@chromium.org>
Date: Wed Oct 18 22:33:37 2017

[Cleanup] Move browsertests to use ScopedFeatureList to modify features

Rietveld Code Review: https://codereview.chromium.org/2910843002/

Bug:  727044 
Change-Id: I64e25142632f42144b66f041ad1c9ffb8ae553f5
Reviewed-on: https://chromium-review.googlesource.com/665657
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509899}
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/chromeos/extensions/input_method_apitest_chromeos.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/media/encrypted_media_supported_types_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/metrics/process_memory_metrics_emitter_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/net/predictor_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/net/predictor_tab_helper.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/password_manager/credential_manager_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/plugins/flash_permission_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/policy/policy_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/ssl/ssl_browser_tests.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/subresource_filter/subresource_filter_browser_test_harness.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/subresource_filter/subresource_filter_worker_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/task_manager/task_manager_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/browser/ui/exclusive_access/fullscreen_controller_interactive_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/common/chrome_features.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/chrome/common/chrome_features.h
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/appcache/appcache_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/generic_sensor_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/media/session/media_session_impl_visibility_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/renderer_host/media/video_capture_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/service_worker/service_worker_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/shared_worker/worker_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/webrtc/webrtc_image_capture_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/browser/webrtc/webrtc_video_capture_browsertest.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/public/test/test_utils.cc
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/public/test/test_utils.h
[modify] https://crrev.com/01e3b250a05ae29fbe847300d4d5a075b17b73af/content/test/BUILD.gn

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52ad7812d7f56e668bc955bc4b0e02fc4766a018

commit 52ad7812d7f56e668bc955bc4b0e02fc4766a018
Author: chaopeng <chaopeng@chromium.org>
Date: Thu Nov 02 18:45:22 2017

Reset FieldTrialList for Browser Tests

A number of browser tests had undefined behavior because
FieldTrailList was not cleaned up before BrowserMain.

This patch is a step stone of using ScopedFeatureList to change
features instead of using CommandLine arguments.

Reset FieldFrielList before calling BrowserMain and add warnings
when FieldTrialParamAssociator fails. Also change
ResourcePrefetchPredictorBrowserTest and
SafeBrowsingBlockingPageBrowserTest to use ScopedFeatureList.

Bug:  727044 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ibe063ce9b4f01bce6de548e5b5a0a67e2d90181e
Reviewed-on: https://chromium-review.googlesource.com/723802
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Reviewed-by: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513565}
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/chrome/browser/predictors/resource_prefetch_predictor_browsertest.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/chrome/browser/signin/dice_browsertest.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/components/variations/field_trial_config/field_trial_util.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/content/browser/frame_host/data_url_navigation_browsertest.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/content/browser/loader/resource_scheduler_browsertest.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/content/public/test/browser_test_base.cc
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/content/public/test/browser_test_base.h
[modify] https://crrev.com/52ad7812d7f56e668bc955bc4b0e02fc4766a018/content/public/test/content_browser_test.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec3d21128b76e21b723d67fd43c23ae9c6235b35

commit ec3d21128b76e21b723d67fd43c23ae9c6235b35
Author: chaopeng <chaopeng@chromium.org>
Date: Wed Nov 08 21:22:22 2017

Move JS BrowserTest to ScopedFeatureList

Background:
Use command line args to change feature will not respect to the
features from bots. So we are moving all test using command line
args to change feature via ScopedFeatureList.

This patch change js2gtest.js to allow it gen code use ScopedFeatureList,
also move the JS BrowserTest to use ScopedFeatureList.

Bug:  727044 
Change-Id: I869aaedc42ab139f61d7c02419a66ec434c26eeb
Reviewed-on: https://chromium-review.googlesource.com/753930
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514941}
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/base/js2gtest.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/extensions/a11y/extensions_a11y_test_fixture.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/extensions/cr_extensions_browsertest.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/md_bookmarks/md_bookmarks_browsertest.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/md_bookmarks/md_bookmarks_focus_test.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/md_user_manager/user_manager_browsertest.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/media/media_engagement_browsertest.js
[modify] https://crrev.com/ec3d21128b76e21b723d67fd43c23ae9c6235b35/chrome/test/data/webui/sys_internals/sys_internals_browsertest.js

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8bd9576f446f4415592be08c7d9e6c48fe7b67cd

commit 8bd9576f446f4415592be08c7d9e6c48fe7b67cd
Author: chaopeng <chaopeng@chromium.org>
Date: Thu Nov 09 04:03:12 2017

[Cleanup] Move PreviewsNoScriptBrowserTest to ScopedFeatureList

Bug:  727044 
Change-Id: Ifd3f6e7e6d9f85940adfc1be51d693c61877c513
Reviewed-on: https://chromium-review.googlesource.com/758937
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515090}
[modify] https://crrev.com/8bd9576f446f4415592be08c7d9e6c48fe7b67cd/chrome/browser/previews/previews_browsertest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment