Revert "[ios] Delays WebTest's BrowserState creation until SetUp()."

This reverts commit 83ca63522c.

Reason for revert: Broke internal waterfall, crbug.com/1207692

Original change's description:
> [ios] Delays WebTest's BrowserState creation until SetUp().
>
> Delaying BrowserState creation until SetUp() will allow subclasses to
> supply their own custom BrowserStates for use in tests. Previously, the
> WebTest constructor was responsible for setting up a TaskEnvironment, so
> it was not possible for a subclass to create a BrowserState before
> WebTest's constructor ran.
>
> Bug: None
> Change-Id: I95d1d8b508d58cc14c11ff81bf7aee10e61d4a4f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2878697
> Commit-Queue: Rohit Rao <rohitrao@chromium.org>
> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#881191}

Bug: None
Change-Id: I08783cb9cd0f9c3634234ae0a012cc3e1e423b85
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2885650
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Javier Flores <javierrobles@chromium.org>
Auto-Submit: Javier Flores <javierrobles@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#881315}

ios/chrome/browser/passwords/password_controller_unittest.mm

@@ -234,14 +234,13 @@ ACTION(InvokeEmptyConsumerWithForms) {
 class PasswordControllerTest : public ChromeWebTest {
  public:
   PasswordControllerTest()
-      : ChromeWebTest(std::make_unique<ChromeWebClient>()) {}
+      : ChromeWebTest(std::make_unique<ChromeWebClient>()),
+        store_(new testing::NiceMock<password_manager::MockPasswordStore>()) {}
 
   ~PasswordControllerTest() override { store_->ShutdownOnUIThread(); }
 
   void SetUp() override {
     ChromeWebTest::SetUp();
-
-    store_ = new testing::NiceMock<password_manager::MockPasswordStore>();
     ON_CALL(*store_, IsAbleToSavePasswords).WillByDefault(Return(true));
 
     // When waiting for predictions is on, it makes tests more complicated.
@@ -1220,12 +1219,12 @@ TEST_F(PasswordControllerTest, SelectingSuggestionShouldFillPasswordForm) {
 // SetUp.
 class PasswordControllerTestSimple : public PlatformTest {
  public:
-  PasswordControllerTestSimple() {}
+  PasswordControllerTestSimple()
+      : store_(new testing::NiceMock<password_manager::MockPasswordStore>()) {}
 
   ~PasswordControllerTestSimple() override { store_->ShutdownOnUIThread(); }
 
   void SetUp() override {
-    store_ = new testing::NiceMock<password_manager::MockPasswordStore>();
     ON_CALL(*store_, IsAbleToSavePasswords).WillByDefault(Return(true));
 
     std::unique_ptr<TestChromeBrowserState> browser_state(builder.Build());
@@ -1288,28 +1287,6 @@ TEST_F(PasswordControllerTestSimple, SaveOnNonHTMLLandingPage) {
   EXPECT_FALSE(form_manager->IsPasswordUpdate());
 }
 
-// Check that if the PasswordController is told (by the PasswordManagerClient)
-// that this is Incognito, it won't enable password generation.
-TEST_F(PasswordControllerTestSimple, IncognitoPasswordGenerationDisabled) {
-  PasswordFormManager::set_wait_for_server_predictions_for_filling(false);
-
-  auto client =
-      std::make_unique<NiceMock<MockPasswordManagerClient>>(store_.get());
-  weak_client_ = client.get();
-
-  EXPECT_CALL(*weak_client_->GetPasswordFeatureManager(), IsGenerationEnabled)
-      .WillRepeatedly(Return(true));
-  EXPECT_CALL(*weak_client_, IsIncognito).WillRepeatedly(Return(true));
-
-  UniqueIDDataTabHelper::CreateForWebState(&web_state_);
-  passwordController_ =
-      [[PasswordController alloc] initWithWebState:&web_state_
-                                            client:std::move(client)];
-
-  EXPECT_FALSE(
-      passwordController_.passwordManagerDriver->GetPasswordGenerationHelper());
-}
-
 // Checks that when the user set a focus on a field of a password form which was
 // not sent to the store then the request the the store is sent.
 TEST_F(PasswordControllerTest, SendingToStoreDynamicallyAddedFormsOnFocus) {
@@ -1673,6 +1650,30 @@ TEST_F(PasswordControllerTest, CheckPasswordGenerationSuggestion) {
 }
 
 
+// Check that if the PasswordController is told (by the PasswordManagerClient)
+// that this is Incognito, it won't enable password generation.
+TEST_F(PasswordControllerTest, IncognitoPasswordGenerationDisabled) {
+    TearDown();
+    ChromeWebTest::SetUp();
+
+    PasswordFormManager::set_wait_for_server_predictions_for_filling(false);
+
+    auto client =
+    std::make_unique<NiceMock<MockPasswordManagerClient>>(store_.get());
+    weak_client_ = client.get();
+
+    EXPECT_CALL(*weak_client_->GetPasswordFeatureManager(), IsGenerationEnabled)
+        .WillRepeatedly(Return(true));
+    EXPECT_CALL(*weak_client_, IsIncognito).WillRepeatedly(Return(true));
+
+    UniqueIDDataTabHelper::CreateForWebState(web_state());
+    passwordController_ =
+    [[PasswordController alloc] initWithWebState:web_state()
+                                          client:std::move(client)];
+
+    EXPECT_FALSE(passwordController_.passwordManagerDriver
+                     ->GetPasswordGenerationHelper());
+}
 
 // Tests that the user is prompted to save or update password on a succesful
 // form submission.

ios/web/download/download_controller_impl_unittest.mm

@@ -30,25 +30,20 @@ const char kMimeType[] = "application/pdf";
 // Test fixture for testing DownloadControllerImpl class.
 class DownloadControllerImplTest : public WebTest {
  protected:
-  DownloadControllerImplTest() {}
-
-  void SetUp() override {
-    WebTest::SetUp();
-
-    download_controller_ = std::make_unique<DownloadControllerImpl>();
-    delegate_ = std::make_unique<FakeDownloadControllerDelegate>(
-        download_controller_.get());
+  DownloadControllerImplTest()
+      : download_controller_(std::make_unique<DownloadControllerImpl>()),
+        delegate_(download_controller_.get()) {
     web_state_.SetBrowserState(GetBrowserState());
   }
 
   FakeWebState web_state_;
   std::unique_ptr<DownloadControllerImpl> download_controller_;
-  std::unique_ptr<FakeDownloadControllerDelegate> delegate_;
+  FakeDownloadControllerDelegate delegate_;
 };
 
 // Tests that DownloadController::GetDelegate returns delegate_.
 TEST_F(DownloadControllerImplTest, Delegate) {
-  ASSERT_EQ(delegate_.get(), download_controller_->GetDelegate());
+  ASSERT_EQ(&delegate_, download_controller_->GetDelegate());
 }
 
 // Tests that DownloadController::FromBrowserState returns the same object for
@@ -72,9 +67,9 @@ TEST_F(DownloadControllerImplTest, OnDownloadCreated) {
                                            @"POST", kContentDisposition,
                                            /*total_bytes=*/-1, kMimeType);
 
-  ASSERT_EQ(1U, delegate_->alive_download_tasks().size());
-  DownloadTask* task = delegate_->alive_download_tasks()[0].second.get();
-  EXPECT_EQ(&web_state_, delegate_->alive_download_tasks()[0].first);
+  ASSERT_EQ(1U, delegate_.alive_download_tasks().size());
+  DownloadTask* task = delegate_.alive_download_tasks()[0].second.get();
+  EXPECT_EQ(&web_state_, delegate_.alive_download_tasks()[0].first);
   EXPECT_NSEQ(identifier, task->GetIndentifier());
   EXPECT_EQ(url, task->GetOriginalUrl());
   EXPECT_NSEQ(@"POST", task->GetHttpMethod());

ios/web/download/download_inttest.mm

@@ -58,12 +58,7 @@ std::unique_ptr<net::test_server::HttpResponse> GetDownloadResponse(
 // DownloadTask integration tests.
 class DownloadTest : public WebTestWithWebState {
  protected:
-  DownloadTest() {}
-
-  void SetUp() override {
-    WebTestWithWebState::SetUp();
-    delegate_ =
-        std::make_unique<FakeDownloadControllerDelegate>(download_controller());
+  DownloadTest() : delegate_(download_controller()) {
     server_.RegisterRequestHandler(base::BindRepeating(&GetDownloadResponse));
   }
 
@@ -73,7 +68,7 @@ class DownloadTest : public WebTestWithWebState {
 
  protected:
   net::EmbeddedTestServer server_;
-  std::unique_ptr<FakeDownloadControllerDelegate> delegate_;
+  FakeDownloadControllerDelegate delegate_;
 };
 
 // Tests sucessfull download flow.
@@ -85,12 +80,12 @@ TEST_F(DownloadTest, SucessfullDownload) {
 
   // Wait until download task is created.
   ASSERT_TRUE(WaitUntilConditionOrTimeout(kWaitForDownloadTimeout, ^{
-    return !delegate_->alive_download_tasks().empty();
+    return !delegate_.alive_download_tasks().empty();
   }));
-  ASSERT_EQ(1U, delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, delegate_.alive_download_tasks().size());
 
   // Verify the initial state of the download task.
-  DownloadTask* task = delegate_->alive_download_tasks()[0].second.get();
+  DownloadTask* task = delegate_.alive_download_tasks()[0].second.get();
   ASSERT_TRUE(task);
   EXPECT_TRUE(task->GetIndentifier());
   EXPECT_EQ(url, task->GetOriginalUrl());

ios/web/find_in_page/find_in_page_manger_impl_unittest.mm

@@ -33,16 +33,16 @@ namespace web {
 // FindInPageManagerDelegate is correct depending on what web frames return.
 class FindInPageManagerImplTest : public WebTest {
  protected:
-  FindInPageManagerImplTest() : WebTest(std::make_unique<FakeWebClient>()) {}
-
-  void SetUp() override {
-    WebTest::SetUp();
-
+  FindInPageManagerImplTest() : WebTest(std::make_unique<FakeWebClient>()) {
     fake_web_state_ = std::make_unique<FakeWebState>();
     fake_web_state_->SetBrowserState(GetBrowserState());
     auto frames_manager = std::make_unique<FakeWebFramesManager>();
     fake_web_frames_manager_ = frames_manager.get();
     fake_web_state_->SetWebFramesManager(std::move(frames_manager));
+  }
+
+  void SetUp() override {
+    WebTest::SetUp();
 
     JavaScriptFeatureManager::FromBrowserState(GetBrowserState())
         ->ConfigureFeatures({FindInPageJavaScriptFeature::GetInstance()});

ios/web/public/test/fakes/fake_download_controller_delegate.h

@@ -36,7 +36,6 @@ class FakeDownloadControllerDelegate : public DownloadControllerDelegate {
                          std::unique_ptr<DownloadTask>) override;
   void OnDownloadControllerDestroyed(DownloadController*) override;
 
-  DownloadController* controller_ = nullptr;
   AliveDownloadTaskList alive_download_tasks_;
 
   DISALLOW_COPY_AND_ASSIGN(FakeDownloadControllerDelegate);

ios/web/public/test/fakes/fake_download_controller_delegate.mm

@@ -4,24 +4,17 @@
 
 #include "ios/web/public/test/fakes/fake_download_controller_delegate.h"
 
-#include "base/check.h"
-#include "base/check_op.h"
 #include "ios/web/public/download/download_controller.h"
 #include "ios/web/public/download/download_task.h"
 
 namespace web {
 
 FakeDownloadControllerDelegate::FakeDownloadControllerDelegate(
-    DownloadController* controller)
-    : controller_(controller) {
-  DCHECK(controller_);
-  controller_->SetDelegate(this);
+    DownloadController* controller) {
+  controller->SetDelegate(this);
 }
 
-FakeDownloadControllerDelegate::~FakeDownloadControllerDelegate() {
-  controller_->SetDelegate(nullptr);
-  controller_ = nullptr;
-}
+FakeDownloadControllerDelegate::~FakeDownloadControllerDelegate() = default;
 
 void FakeDownloadControllerDelegate::OnDownloadCreated(
     DownloadController* download_controller,
@@ -32,9 +25,7 @@ void FakeDownloadControllerDelegate::OnDownloadCreated(
 
 void FakeDownloadControllerDelegate::OnDownloadControllerDestroyed(
     DownloadController* controller) {
-  DCHECK_EQ(controller_, controller);
   controller->SetDelegate(nullptr);
-  controller_ = nullptr;
 }
 
 }  // namespace web

ios/web/public/test/web_test.h

@@ -29,13 +29,6 @@ class WebTest : public PlatformTest {
           WebTaskEnvironment::Options = WebTaskEnvironment::Options::DEFAULT);
   ~WebTest() override;
 
-  void SetUp() override;
-
-  // Creates and returns a BrowserState for use in tests. The default
-  // implementation returns a FakeBrowserState, but subclasses can override this
-  // to supply a custom BrowserState.
-  virtual std::unique_ptr<BrowserState> CreateBrowserState();
-
   // Manually overrides the built in JavaScriptFeatures and those from
   // |GetWebClient()::GetJavaScriptFeatures()|. This is intended to be used to
   // replace an instance of a built in feature with one created by the test.

ios/web/public/test/web_test.mm

@@ -35,23 +35,11 @@ WebTest::WebTest(std::unique_ptr<web::WebClient> web_client,
                  WebTaskEnvironment::Options options)
     : web_client_(std::move(web_client)),
       task_environment_(options),
-      // browser_state_(std::make_unique<FakeBrowserState>()),
+      browser_state_(std::make_unique<FakeBrowserState>()),
       crash_observer_(std::make_unique<WebTestRenderProcessCrashObserver>()) {}
 
 WebTest::~WebTest() {}
 
-void WebTest::SetUp() {
-  PlatformTest::SetUp();
-
-  DCHECK(!browser_state_);
-  browser_state_ = CreateBrowserState();
-  DCHECK(browser_state_);
-}
-
-std::unique_ptr<BrowserState> WebTest::CreateBrowserState() {
-  return std::make_unique<FakeBrowserState>();
-}
-
 void WebTest::OverrideJavaScriptFeatures(
     std::vector<JavaScriptFeature*> features) {
   WKWebViewConfigurationProvider& configuration_provider =

ios/web/web_state/ui/crw_web_controller_unittest.mm

@@ -519,13 +519,7 @@ TEST_F(CRWWebControllerJSExecutionTest, WindowIdMissmatch) {
 // delegate method.
 class CRWWebControllerResponseTest : public CRWWebControllerTest {
  protected:
-  CRWWebControllerResponseTest() {}
-
-  void SetUp() override {
-    CRWWebControllerTest::SetUp();
-    download_delegate_ =
-        std::make_unique<FakeDownloadControllerDelegate>(download_controller());
-  }
+  CRWWebControllerResponseTest() : download_delegate_(download_controller()) {}
 
   // Calls webView:decidePolicyForNavigationResponse:decisionHandler: callback
   // and waits for decision handler call. Returns false if decision handler call
@@ -564,7 +558,7 @@ class CRWWebControllerResponseTest : public CRWWebControllerTest {
     return DownloadController::FromBrowserState(GetBrowserState());
   }
 
-  std::unique_ptr<FakeDownloadControllerDelegate> download_delegate_;
+  FakeDownloadControllerDelegate download_delegate_;
 };
 
 // Tests that webView:decidePolicyForNavigationResponse:decisionHandler: allows
@@ -582,7 +576,7 @@ TEST_F(CRWWebControllerResponseTest, AllowRendererInitiatedResponse) {
   EXPECT_EQ(WKNavigationResponsePolicyAllow, policy);
 
   // Verify that download task was not created for html response.
-  ASSERT_TRUE(download_delegate_->alive_download_tasks().empty());
+  ASSERT_TRUE(download_delegate_.alive_download_tasks().empty());
 }
 
 // Tests that webView:decidePolicyForNavigationResponse:decisionHandler: allows
@@ -602,7 +596,7 @@ TEST_F(CRWWebControllerResponseTest,
   EXPECT_EQ(WKNavigationResponsePolicyAllow, policy);
 
   // Verify that download task was not created for html response.
-  ASSERT_TRUE(download_delegate_->alive_download_tasks().empty());
+  ASSERT_TRUE(download_delegate_.alive_download_tasks().empty());
 }
 
 // Tests that webView:decidePolicyForNavigationResponse:decisionHandler: blocks
@@ -623,9 +617,9 @@ TEST_F(CRWWebControllerResponseTest,
   EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
 
   // Verify that download task was created (see crbug.com/949114).
-  ASSERT_EQ(1U, download_delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
   DownloadTask* task =
-      download_delegate_->alive_download_tasks()[0].second.get();
+      download_delegate_.alive_download_tasks()[0].second.get();
   ASSERT_TRUE(task);
   EXPECT_TRUE(task->GetIndentifier());
   EXPECT_EQ(kTestDataURL, task->GetOriginalUrl());
@@ -655,7 +649,7 @@ TEST_F(CRWWebControllerResponseTest,
   EXPECT_EQ(WKNavigationResponsePolicyAllow, policy);
 
   // Verify that download task was not created for html response.
-  ASSERT_TRUE(download_delegate_->alive_download_tasks().empty());
+  ASSERT_TRUE(download_delegate_.alive_download_tasks().empty());
 }
 
 // Tests that webView:decidePolicyForNavigationResponse:decisionHandler:
@@ -681,9 +675,9 @@ TEST_F(CRWWebControllerResponseTest, DownloadForPostRequest) {
   EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
 
   // Verify that download task was created with POST method (crbug.com/.
-  ASSERT_EQ(1U, download_delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
   DownloadTask* task =
-      download_delegate_->alive_download_tasks()[0].second.get();
+      download_delegate_.alive_download_tasks()[0].second.get();
   ASSERT_TRUE(task);
   EXPECT_TRUE(task->GetIndentifier());
   EXPECT_NSEQ(@"POST", task->GetHttpMethod());
@@ -705,9 +699,9 @@ TEST_F(CRWWebControllerResponseTest, DownloadWithNSURLResponse) {
   EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
 
   // Verify that download task was created.
-  ASSERT_EQ(1U, download_delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
   DownloadTask* task =
-      download_delegate_->alive_download_tasks()[0].second.get();
+      download_delegate_.alive_download_tasks()[0].second.get();
   ASSERT_TRUE(task);
   EXPECT_TRUE(task->GetIndentifier());
   EXPECT_EQ(kTestURLString, task->GetOriginalUrl());
@@ -734,9 +728,9 @@ TEST_F(CRWWebControllerResponseTest, DownloadWithNSHTTPURLResponse) {
   EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
 
   // Verify that download task was created.
-  ASSERT_EQ(1U, download_delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
   DownloadTask* task =
-      download_delegate_->alive_download_tasks()[0].second.get();
+      download_delegate_.alive_download_tasks()[0].second.get();
   ASSERT_TRUE(task);
   EXPECT_TRUE(task->GetIndentifier());
   EXPECT_EQ(kTestURLString, task->GetOriginalUrl());
@@ -763,7 +757,7 @@ TEST_F(CRWWebControllerResponseTest, DownloadDiscardsPendingUrl) {
   EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
 
   // Verify that download task was created and pending URL discarded.
-  ASSERT_EQ(1U, download_delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
   EXPECT_EQ("", web_state()->GetVisibleURL());
 }
 
@@ -785,9 +779,9 @@ TEST_F(CRWWebControllerResponseTest, IFrameDownloadWithNSHTTPURLResponse) {
   EXPECT_EQ(WKNavigationResponsePolicyCancel, policy);
 
   // Verify that download task was created.
-  ASSERT_EQ(1U, download_delegate_->alive_download_tasks().size());
+  ASSERT_EQ(1U, download_delegate_.alive_download_tasks().size());
   DownloadTask* task =
-      download_delegate_->alive_download_tasks()[0].second.get();
+      download_delegate_.alive_download_tasks()[0].second.get();
   ASSERT_TRUE(task);
   EXPECT_TRUE(task->GetIndentifier());
   EXPECT_EQ(kTestURLString, task->GetOriginalUrl());

ios/web/web_state/web_state_impl_unittest.mm

@@ -159,10 +159,7 @@ void HandleScriptCommand(bool* is_called,
 // Test fixture for web::WebStateImpl class.
 class WebStateImplTest : public web::WebTest {
  protected:
-  WebStateImplTest() = default;
-
-  void SetUp() override {
-    web::WebTest::SetUp();
+  WebStateImplTest() : web::WebTest() {
     web::WebState::CreateParams params(GetBrowserState());
     web_state_ = std::make_unique<web::WebStateImpl>(params);
   }