Simplify how TestChromeWebUIControllerFactory is registered now that multiple WebUIControllerFactor…

chrome/test/base/web_ui_browsertest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/ui/webui/web_ui_browsertest.h"
+#include "chrome/test/base/web_ui_browsertest.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/path_service.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/printing/print_preview_dialog_controller.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
-#include "chrome/browser/ui/webui/test_chrome_web_ui_controller_factory.h"
 #include "chrome/browser/ui/webui/web_ui_test_handler.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/test/base/test_chrome_web_ui_controller_factory.h"
 #include "chrome/test/base/test_tab_strip_model_observer.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/url_data_source.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/browser/web_ui_message_handler.h"
 #include "net/base/net_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest-spi.h"
@@ skipping 276 matching lines @@
  private:
   DISALLOW_COPY_AND_ASSIGN(MockWebUIProvider);
 };
 
 base::LazyInstance<MockWebUIProvider> mock_provider_ =
     LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 void WebUIBrowserTest::SetUpOnMainThread() {
-  InProcessBrowserTest::SetUpOnMainThread();
+  logging::SetLogMessageHandler(&LogHandler);
 
-  logging::SetLogMessageHandler(&LogHandler);
-}
+  content::WebUIControllerFactory::UnregisterFactoryForTesting(
+      ChromeWebUIControllerFactory::GetInstance());

[scr, 2013/01/29 01:01:54]

See comment below - would it be easier to use if this were more like a stack and
we didn't have to unregister the previous one, just register/unregister ours?

[scr, 2013/01/29 01:30:10]

Per IM, the goal is to get to multiple non-overlapping factories registered, so
the answer is no - it shouldn't be a stack.  That said, it would be nice if the
main factory were implemented with some means to populate a hash rather than
walking each of its known types and individual factories could register what
they knew.

If we had that, then you could see each host -> WebUIMaker be more like a stack,
where you could override temporarily.

FWIW, I added this only in this factory originally because I didn't want to add
any complexity to the non-test runtime, but seeing how the runtime does that big
if if if block in its GetWebUIFactoryFunction, I think the eventual overhaul to
support this natively (ditching this specialty class altogether) would save
cycles in the long run and be easier to understand.

 
-void WebUIBrowserTest::CleanUpOnMainThread() {
-  InProcessBrowserTest::CleanUpOnMainThread();
+  test_factory_.reset(new TestChromeWebUIControllerFactory);
 
-  logging::SetLogMessageHandler(NULL);
-}
+  content::WebUIControllerFactory::RegisterFactory(test_factory_.get());
 
-void WebUIBrowserTest::SetUpInProcessBrowserTestFixture() {
-  InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
-  TestChromeWebUIControllerFactory::AddFactoryOverride(
+  test_factory_->AddFactoryOverride(
       GURL(kDummyURL).host(), mock_provider_.Pointer());
 
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &test_data_directory_));
   test_data_directory_ = test_data_directory_.Append(kWebUITestFolder);
   ASSERT_TRUE(PathService::Get(chrome::DIR_GEN_TEST_DATA,
                                &gen_test_data_directory_));
 
   // TODO(dtseng): should this be part of every BrowserTest or just WebUI test.
   FilePath resources_pack_path;
   PathService::Get(chrome::FILE_RESOURCES_PACK, &resources_pack_path);
   ResourceBundle::GetSharedInstance().AddDataPackFromPath(
       resources_pack_path, ui::SCALE_FACTOR_NONE);
 
   AddLibraryFromSourceRoot(this, FilePath(kA11yAuditLibraryJSPath));
   AddLibraryFromSourceRoot(this, FilePath(kMockJSPath));
   AddLibrary(FilePath(kWebUILibraryJS));
 }
 
-void WebUIBrowserTest::TearDownInProcessBrowserTestFixture() {
-  InProcessBrowserTest::TearDownInProcessBrowserTestFixture();
-  TestChromeWebUIControllerFactory::RemoveFactoryOverride(
-      GURL(kDummyURL).host());
+void WebUIBrowserTest::CleanUpOnMainThread() {
+  logging::SetLogMessageHandler(NULL);
+
+  test_factory_->RemoveFactoryOverride(GURL(kDummyURL).host());
+  content::WebUIControllerFactory::UnregisterFactoryForTesting(
+      test_factory_.get());

[scr, 2013/01/29 01:01:54]

Should the old one be put back?  If so, would some sort of stack registry be
easier - you could just register & unregister yours without caring/knowing what
was there before…

[scr, 2013/01/29 01:30:10]

Neg - read above.

+
+  test_factory_.reset();
 }
 
 void WebUIBrowserTest::SetWebUIInstance(content::WebUI* web_ui) {
   override_selected_web_ui_ = web_ui;
 }
 
 WebUIMessageHandler* WebUIBrowserTest::GetMockMessageHandler() {
   return NULL;
 }
 
@@ skipping 342 matching lines @@
 // testDone directly and expect pass result.
 IN_PROC_BROWSER_TEST_F(WebUIBrowserAsyncTest, TestTestDoneEarlyPassesAsync) {
   ASSERT_TRUE(RunJavascriptAsyncTest("testDone"));
 }
 
 // Test that calling testDone during RunJavascriptTest still completes when
 // waiting for async result.
 IN_PROC_BROWSER_TEST_F(WebUIBrowserAsyncTest, TestTestDoneEarlyPasses) {
   ASSERT_TRUE(RunJavascriptTest("testDone"));
 }