[Cleanup] Move all browsertests to use ScopedFeatureList to modify features

chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc

Index: chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc
diff --git a/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc
index 01535a44e2a68383550108db0c3e52cf79810967..54ce8dc788db5b25d33a244bf514f9718894a2b3 100644
--- a/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc
+++ b/chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc
@@ -7,6 +7,8 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/test/histogram_tester.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/common/content_switches.h"
@@ -16,21 +18,23 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
-const char kAdsMetricsFeature[] = "AdsMetrics";
-
 class AdsPageLoadMetricsObserverBrowserTest : public InProcessBrowserTest {
  public:
-  AdsPageLoadMetricsObserverBrowserTest() {
-    base::CommandLine* cmd_line = base::CommandLine::ForCurrentProcess();
-    cmd_line->AppendSwitchASCII(switches::kEnableFeatures, kAdsMetricsFeature);
-  }
+  AdsPageLoadMetricsObserverBrowserTest() {}
+

[Ilya Sherman, 2017/07/03 21:21:59]

nit: Please omit this blank line.

[chaopeng, 2017/07/14 18:43:50]

Done.

   ~AdsPageLoadMetricsObserverBrowserTest() override {}
 
+  void SetUp() override {
+    scoped_feature_list_.InitAndEnableFeature(features::kAdsFeature);

[Ilya Sherman, 2017/07/03 21:21:59]

Hmm, if this runs before InProcessBrowserTest::SetUp(), is it important to do it
as part of SetUp(), or could it be part of the constructor?  The constructor is
always preferred when possible, as it's the simplest to reason about.

[chaopeng, 2017/07/14 18:43:50]

Move all calls to constructor.

+    InProcessBrowserTest::SetUp();
+  }
+
   void SetUpOnMainThread() override {
     ASSERT_TRUE(embedded_test_server()->Start());
   }
 
  private:
+  base::test::ScopedFeatureList scoped_feature_list_;
   DISALLOW_COPY_AND_ASSIGN(AdsPageLoadMetricsObserverBrowserTest);
 };