Avoid SyncConfirmationHandler unittest creating its own Profile

The SyncConfirmationHandler unittest inherits from
BrowserWithTestWindowTest. It currently overrides the latter's
CreateProfile() method to create and own a Profile instance via
IdentityTestEnvironmentProfileAdaptor. However, this blocks the
upcoming elimination of FakeSigninManager, via a subtle circular
dependency that gets introduced:
- SigninManagerFactory supplies |g_browser_process->local_state()| to
  SigninManager's Initialize() method.
- In the testing context, the object thus supplied is actually owned by
  BrowserWithTestWindowTest's TestingProfileManager object.
- Hence, the Profile of the question must be torn down before the
  TestingProfileManager to avoid a crash that otherwise occurs in
  SigninManager::Shutdown() when it accesses the PrefService pointer
  that it has cached.
- However, it is not possible to move the teardown of the
  SyncConfirmationHandler unittest's |profile_| instance to be before
  the call to BrowserWithTestWindowTest::TearDown(), as that in turn
  causes a crash because other objects that BrowserWithTestWindowTest
  owns (namely, |browser_|) must be torn down *before* the Profile.

This CL fixes the problem by simply eliminating SyncConfirmationHandler
owning its own Profile instance. Instead, it just supplies the factories
that IdentityTestEnvironmentProfileAdaptor requires directly to
BrowserWithTestWindowTest via an override of GetTestingFactories(). This
allows the natural teardown order in
BrowserWithTestWindowTest::TearDown() to be preserved.

Bug: 796544
Change-Id: I06528d564d836d83e1ac87a6cfc94c9ca12a033c
Reviewed-on: https://chromium-review.googlesource.com/c/1488872Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: Colin Blundell <blundell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635485}

chrome/browser/ui/webui/signin/sync_confirmation_handler_unittest.cc

@@ -105,7 +105,6 @@ class SyncConfirmationHandlerTest : public BrowserWithTestWindowTest,
     web_ui_.reset();
     identity_test_env_adaptor_.reset();
     BrowserWithTestWindowTest::TearDown();
-    profile_.reset();
 
     EXPECT_EQ(did_user_explicitly_interact ? 0 : 1,
               user_action_tester()->GetActionCount("Signin_Abort_Signin"));
@@ -130,21 +129,17 @@ class SyncConfirmationHandlerTest : public BrowserWithTestWindowTest,
     return identity_test_env_adaptor_->identity_test_env();
   }
 
-  // BrowserWithTestWindowTest:
-  TestingProfile* CreateProfile() override {
-    profile_ = IdentityTestEnvironmentProfileAdaptor::
-        CreateProfileForIdentityTestEnvironment(GetTestingFactories());
-    return profile_.get();
-  }
-
   BrowserWindow* CreateBrowserWindow() override {
     return new DialogTestBrowserWindow;
   }
 
   TestingProfile::TestingFactories GetTestingFactories() override {
-    return {
+    TestingProfile::TestingFactories factories = {
         {ConsentAuditorFactory::GetInstance(),
          base::BindRepeating(&BuildFakeConsentAuditor)}};
+    IdentityTestEnvironmentProfileAdaptor::
+        AppendIdentityTestEnvironmentFactories(&factories);
+    return factories;
   }
 
   const std::unordered_map<std::string, int>& GetStringToGrdIdMap() {
@@ -205,7 +200,6 @@ class SyncConfirmationHandlerTest : public BrowserWithTestWindowTest,
   ScopedObserver<LoginUIService, LoginUIService::Observer>
       login_ui_service_observer_;
   base::HistogramTester histogram_tester_;
-  std::unique_ptr<TestingProfile> profile_;
   std::unique_ptr<IdentityTestEnvironmentProfileAdaptor>
       identity_test_env_adaptor_;