Change CreateWebUIControllerForURL() to return a unique_ptr.

In WebUIControllerFactory and subclasses.

Change-Id: I93f7d17265bfb77f402eef4bd08566614393b21c
Reviewed-on: https://chromium-review.googlesource.com/1087627Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565142}

android_webview/browser/aw_web_ui_controller_factory.cc

@@ -4,6 +4,7 @@
 
 #include "android_webview/browser/aw_web_ui_controller_factory.h"
 
+#include "base/memory/ptr_util.h"
 #include "components/safe_browsing/web_ui/constants.h"
 #include "components/safe_browsing/web_ui/safe_browsing_ui.h"
 #include "content/public/browser/web_ui.h"
@@ -73,14 +74,14 @@ bool AwWebUIControllerFactory::UseWebUIBindingsForURL(
   return UseWebUIForURL(browser_context, url);
 }
 
-WebUIController* AwWebUIControllerFactory::CreateWebUIControllerForURL(
-    WebUI* web_ui,
+std::unique_ptr<WebUIController>
+AwWebUIControllerFactory::CreateWebUIControllerForURL(WebUI* web_ui,
                                                       const GURL& url) const {
   WebUIFactoryFunctionPointer function = GetWebUIFactoryFunctionPointer(url);
   if (!function)
     return nullptr;
 
-  return (*function)(web_ui, url);
+  return base::WrapUnique((*function)(web_ui, url));
 }
 
 }  // namespace android_webview

android_webview/browser/aw_web_ui_controller_factory.h

@@ -22,7 +22,7 @@ class AwWebUIControllerFactory : public content::WebUIControllerFactory {
                       const GURL& url) const override;
   bool UseWebUIBindingsForURL(content::BrowserContext* browser_context,
                               const GURL& url) const override;
-  content::WebUIController* CreateWebUIControllerForURL(
+  std::unique_ptr<content::WebUIController> CreateWebUIControllerForURL(
       content::WebUI* web_ui,
       const GURL& url) const override;
 

chrome/browser/ui/webui/chrome_web_ui_controller_factory.cc

@@ -11,6 +11,7 @@
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/about_flags.h"
@@ -681,18 +682,19 @@ bool ChromeWebUIControllerFactory::UseWebUIBindingsForURL(
   return UseWebUIForURL(browser_context, url);
 }
 
-WebUIController* ChromeWebUIControllerFactory::CreateWebUIControllerForURL(
+std::unique_ptr<WebUIController>
+ChromeWebUIControllerFactory::CreateWebUIControllerForURL(
     WebUI* web_ui,
     const GURL& url) const {
   Profile* profile = Profile::FromWebUI(web_ui);
   WebUIFactoryFunction function = GetWebUIFactoryFunction(web_ui, profile, url);
   if (!function)
-    return NULL;
+    return nullptr;
 
-  if (web_ui->GetWebContents()->GetMainFrame() != nullptr)
+  if (web_ui->GetWebContents()->GetMainFrame())
     webui::LogWebUIUrl(url);
 
-  return (*function)(web_ui, url);
+  return base::WrapUnique((*function)(web_ui, url));
 }
 
 void ChromeWebUIControllerFactory::GetFaviconForURL(

chrome/browser/ui/webui/chrome_web_ui_controller_factory.h

@@ -20,13 +20,16 @@ class RefCountedMemory;
 
 class ChromeWebUIControllerFactory : public content::WebUIControllerFactory {
  public:
+  static ChromeWebUIControllerFactory* GetInstance();
+
+  // content::WebUIControllerFactory:
   content::WebUI::TypeID GetWebUIType(content::BrowserContext* browser_context,
                                       const GURL& url) const override;
   bool UseWebUIForURL(content::BrowserContext* browser_context,
                       const GURL& url) const override;
   bool UseWebUIBindingsForURL(content::BrowserContext* browser_context,
                               const GURL& url) const override;
-  content::WebUIController* CreateWebUIControllerForURL(
+  std::unique_ptr<content::WebUIController> CreateWebUIControllerForURL(
       content::WebUI* web_ui,
       const GURL& url) const override;
 
@@ -38,8 +41,6 @@ class ChromeWebUIControllerFactory : public content::WebUIControllerFactory {
       const std::vector<int>& desired_sizes_in_pixel,
       const favicon_base::FaviconResultsCallback& callback) const;
 
-  static ChromeWebUIControllerFactory* GetInstance();
-
  protected:
   ChromeWebUIControllerFactory();
   ~ChromeWebUIControllerFactory() override;

chrome/test/base/test_chrome_web_ui_controller_factory.cc

@@ -4,8 +4,10 @@
 
 #include "chrome/test/base/test_chrome_web_ui_controller_factory.h"
 
+#include "base/memory/ptr_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_ui_controller.h"
 
 using content::WebContents;
 using content::WebUI;
@@ -40,12 +42,15 @@ WebUI::TypeID TestChromeWebUIControllerFactory::GetWebUIType(
       ChromeWebUIControllerFactory::GetWebUIType(profile, url);
 }
 
-WebUIController* TestChromeWebUIControllerFactory::CreateWebUIControllerForURL(
-    content::WebUI* web_ui, const GURL& url) const {
+std::unique_ptr<WebUIController>
+TestChromeWebUIControllerFactory::CreateWebUIControllerForURL(
+    content::WebUI* web_ui,
+    const GURL& url) const {
   Profile* profile = Profile::FromWebUI(web_ui);
   WebUIProvider* provider = GetWebUIProvider(profile, url);
-  return provider ? provider->NewWebUI(web_ui, url) :
-      ChromeWebUIControllerFactory::CreateWebUIControllerForURL(web_ui, url);
+  return provider ? base::WrapUnique(provider->NewWebUI(web_ui, url))
+                  : ChromeWebUIControllerFactory::CreateWebUIControllerForURL(
+                        web_ui, url);
 }
 
 TestChromeWebUIControllerFactory::WebUIProvider*
@@ -53,5 +58,5 @@ TestChromeWebUIControllerFactory::WebUIProvider*
         Profile* profile, const GURL& url) const {
   FactoryOverridesMap::const_iterator found =
       factory_overrides_.find(url.host());
-  return (found == factory_overrides_.end()) ? NULL : found->second;
+  return found != factory_overrides_.end() ? found->second : nullptr;
 }

chrome/test/base/test_chrome_web_ui_controller_factory.h

@@ -44,7 +44,7 @@ class TestChromeWebUIControllerFactory : public ChromeWebUIControllerFactory {
   // ChromeWebUIFactory overrides.
   content::WebUI::TypeID GetWebUIType(content::BrowserContext* browser_context,
                                       const GURL& url) const override;
-  content::WebUIController* CreateWebUIControllerForURL(
+  std::unique_ptr<content::WebUIController> CreateWebUIControllerForURL(
       content::WebUI* web_ui,
       const GURL& url) const override;
 

content/browser/frame_host/render_frame_host_manager_unittest.cc

@@ -131,12 +131,13 @@ class RenderFrameHostManagerTestWebUIControllerFactory
   }
 
   // WebUIFactory implementation.
-  WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
+  std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
       const GURL& url) const override {
     // If WebUI creation is enabled for the test and this is a WebUI URL,
     // returns a new instance.
     if (should_create_webui_ && HasWebUIScheme(url))
-      return new WebUIController(web_ui);
+      return std::make_unique<WebUIController>(web_ui);
     return nullptr;
   }
 

content/browser/network_service_browsertest.cc

@@ -56,12 +56,14 @@ class RenderProcessKilledObserver : public WebContentsObserver {
 
 class WebUITestWebUIControllerFactory : public WebUIControllerFactory {
  public:
-  WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
+  std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
       const GURL& url) const override {
     std::string foo(url.path());
     if (url.path() == "/nobinding/")
       web_ui->SetBindings(0);
-    return HasWebUIScheme(url) ? new WebUIController(web_ui) : nullptr;
+    return HasWebUIScheme(url) ? std::make_unique<WebUIController>(web_ui)
+                               : nullptr;
   }
   WebUI::TypeID GetWebUIType(BrowserContext* browser_context,
                              const GURL& url) const override {

content/browser/service_worker/service_worker_internals_ui.h

@@ -31,12 +31,12 @@ class ServiceWorkerInternalsUI
   using StatusCallback = base::OnceCallback<void(ServiceWorkerStatusCode)>;
 
   explicit ServiceWorkerInternalsUI(WebUI* web_ui);
+  ~ServiceWorkerInternalsUI() override;
 
  private:
   class OperationProxy;
   class PartitionObserver;
 
-  ~ServiceWorkerInternalsUI() override;
   void AddContextFromStoragePartition(StoragePartition* partition);
 
   void RemoveObserverFromStoragePartition(StoragePartition* partition);

content/browser/webui/content_web_ui_controller_factory.cc

@@ -55,33 +55,35 @@ bool ContentWebUIControllerFactory::UseWebUIBindingsForURL(
   return UseWebUIForURL(browser_context, url);
 }
 
-WebUIController* ContentWebUIControllerFactory::CreateWebUIControllerForURL(
-    WebUI* web_ui, const GURL& url) const {
+std::unique_ptr<WebUIController>
+ContentWebUIControllerFactory::CreateWebUIControllerForURL(
+    WebUI* web_ui,
+    const GURL& url) const {
   if (!url.SchemeIs(kChromeUIScheme))
     return nullptr;
 
   if (url.host_piece() == kChromeUIAppCacheInternalsHost)
-    return new AppCacheInternalsUI(web_ui);
+    return std::make_unique<AppCacheInternalsUI>(web_ui);
   if (url.host_piece() == kChromeUIGpuHost)
-    return new GpuInternalsUI(web_ui);
+    return std::make_unique<GpuInternalsUI>(web_ui);
   if (url.host_piece() == kChromeUIHistogramHost)
-    return new HistogramsInternalsUI(web_ui);
+    return std::make_unique<HistogramsInternalsUI>(web_ui);
   if (url.host_piece() == kChromeUIIndexedDBInternalsHost)
-    return new IndexedDBInternalsUI(web_ui);
+    return std::make_unique<IndexedDBInternalsUI>(web_ui);
   if (url.host_piece() == kChromeUIMediaInternalsHost)
-    return new MediaInternalsUI(web_ui);
+    return std::make_unique<MediaInternalsUI>(web_ui);
   if (url.host_piece() == kChromeUIAccessibilityHost)
-    return new AccessibilityUI(web_ui);
+    return std::make_unique<AccessibilityUI>(web_ui);
   if (url.host_piece() == kChromeUIServiceWorkerInternalsHost)
-    return new ServiceWorkerInternalsUI(web_ui);
+    return std::make_unique<ServiceWorkerInternalsUI>(web_ui);
   if (url.host_piece() == kChromeUINetworkErrorsListingHost)
-    return new NetworkErrorsListingUI(web_ui);
+    return std::make_unique<NetworkErrorsListingUI>(web_ui);
 #if !defined(OS_ANDROID)
   if (url.host_piece() == kChromeUITracingHost)
-    return new TracingUI(web_ui);
+    return std::make_unique<TracingUI>(web_ui);
 #endif
   if (url.host_piece() == kChromeUIWebRTCInternalsHost)
-    return new WebRTCInternalsUI(web_ui);
+    return std::make_unique<WebRTCInternalsUI>(web_ui);
 
   return nullptr;
 }

content/browser/webui/content_web_ui_controller_factory.h

@@ -15,17 +15,19 @@ namespace content {
 class CONTENT_EXPORT ContentWebUIControllerFactory
     : public WebUIControllerFactory {
  public:
+  static ContentWebUIControllerFactory* GetInstance();
+
+  // WebUIControllerFactory:
   WebUI::TypeID GetWebUIType(BrowserContext* browser_context,
                              const GURL& url) const override;
   bool UseWebUIForURL(BrowserContext* browser_context,
                       const GURL& url) const override;
   bool UseWebUIBindingsForURL(BrowserContext* browser_context,
                               const GURL& url) const override;
-  WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
+  std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
       const GURL& url) const override;
 
-  static ContentWebUIControllerFactory* GetInstance();
-
  protected:
   ContentWebUIControllerFactory();
   ~ContentWebUIControllerFactory() override;

content/browser/webui/web_ui_controller_factory_registry.cc

@@ -9,6 +9,7 @@
 #include "base/lazy_instance.h"
 #include "content/browser/frame_host/debug_urls.h"
 #include "content/public/browser/content_browser_client.h"
+#include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
 #include "url/gurl.h"
@@ -39,13 +40,14 @@ WebUIControllerFactoryRegistry* WebUIControllerFactoryRegistry::GetInstance() {
   return base::Singleton<WebUIControllerFactoryRegistry>::get();
 }
 
-WebUIController* WebUIControllerFactoryRegistry::CreateWebUIControllerForURL(
-    WebUI* web_ui, const GURL& url) const {
-  std::vector<WebUIControllerFactory*>* factories =
-      g_web_ui_controller_factories.Pointer();
-  for (size_t i = 0; i < factories->size(); ++i) {
-    WebUIController* controller = (*factories)[i]->CreateWebUIControllerForURL(
-        web_ui, url);
+std::unique_ptr<WebUIController>
+WebUIControllerFactoryRegistry::CreateWebUIControllerForURL(
+    WebUI* web_ui,
+    const GURL& url) const {
+  std::vector<WebUIControllerFactory*>& factories =
+      g_web_ui_controller_factories.Get();
+  for (const WebUIControllerFactory* factory : factories) {
+    auto controller = factory->CreateWebUIControllerForURL(web_ui, url);
     if (controller)
       return controller;
   }

content/browser/webui/web_ui_controller_factory_registry.h

@@ -20,7 +20,8 @@ class CONTENT_EXPORT WebUIControllerFactoryRegistry
 
   // WebUIControllerFactory implementation. Each method loops through the same
   // method on all the factories.
-  WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
+  std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
       const GURL& url) const override;
   WebUI::TypeID GetWebUIType(BrowserContext* browser_context,
                              const GURL& url) const override;

content/browser/webui/web_ui_mojo_browsertest.cc

@@ -160,19 +160,22 @@ class TestWebUIControllerFactory : public WebUIControllerFactory {
 
   void set_run_loop(base::RunLoop* run_loop) { run_loop_ = run_loop; }
 
-  WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
+  std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
       const GURL& url) const override {
     if (url.query() == "ping")
-      return new PingTestWebUIController(web_ui, run_loop_);
-    if (url.query() == "webui_bindings")
-      return new TestWebUIController(web_ui, run_loop_, BINDINGS_POLICY_WEB_UI);
+      return std::make_unique<PingTestWebUIController>(web_ui, run_loop_);
+    if (url.query() == "webui_bindings") {
+      return std::make_unique<TestWebUIController>(web_ui, run_loop_,
+                                                   BINDINGS_POLICY_WEB_UI);
+    }
     if (url.query() == "hybrid") {
-      return new TestWebUIController(
+      return std::make_unique<TestWebUIController>(
           web_ui, run_loop_,
           BINDINGS_POLICY_WEB_UI | BINDINGS_POLICY_MOJO_WEB_UI);
     }
 
-    return new TestWebUIController(web_ui, run_loop_);
+    return std::make_unique<TestWebUIController>(web_ui, run_loop_);
   }
   WebUI::TypeID GetWebUIType(BrowserContext* browser_context,
                              const GURL& url) const override {

content/public/browser/web_ui_controller_factory.h

@@ -5,6 +5,8 @@
 #ifndef CONTENT_PUBLIC_BROWSER_WEB_UI_CONTROLLER_FACTORY_H_
 #define CONTENT_PUBLIC_BROWSER_WEB_UI_CONTROLLER_FACTORY_H_
 
+#include <memory>
+
 #include "content/common/content_export.h"
 #include "content/public/browser/web_ui.h"
 
@@ -28,8 +30,9 @@ class CONTENT_EXPORT WebUIControllerFactory {
 
   // Returns a WebUIController instance for the given URL, or nullptr if the URL
   // doesn't correspond to a WebUI.
-  virtual WebUIController* CreateWebUIControllerForURL(
-      WebUI* web_ui, const GURL& url) const = 0;
+  virtual std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
+      const GURL& url) const = 0;
 
   // Gets the WebUI type for the given URL. This will return kNoWebUI if the
   // corresponding call to CreateWebUIForURL would fail, or something

content/renderer/render_view_browsertest.cc

@@ -29,6 +29,7 @@
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/native_web_keyboard_event.h"
+#include "content/public/browser/web_ui_controller.h"
 #include "content/public/browser/web_ui_controller_factory.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/browser_side_navigation_policy.h"
@@ -160,7 +161,8 @@ int ConvertMockKeyboardModifier(MockKeyboard::Modifiers modifiers) {
 
 class WebUITestWebUIControllerFactory : public WebUIControllerFactory {
  public:
-  WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
+  std::unique_ptr<WebUIController> CreateWebUIControllerForURL(
+      WebUI* web_ui,
       const GURL& url) const override {
     return nullptr;
   }