Get rid of WebUIDataSource::Create() and WebUIDataSource::Add()

- Create() has been replaced by CreateAndAdd().
- Replace the last Create() caller in WebUIDataSourceTest with a direct
  WebUIDataSourceImpl allocation.
- Fold the 1 line Add() implementation into CreateAndAdd().
- Implement TestWebUIDataSource::AddDataSourceForBrowserContext().
- Update the last Add() caller, which is in a unit test, to use
  TestWebUIDataSource::AddDataSourceForBrowserContext().

Bug: 1206140
Change-Id: Ief1102c8771f736f4f5fcbb597084434e9ed6220
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4349773
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1119020}

chrome/browser/ui/webui/managed_ui_handler_unittest.cc

@@ -43,10 +43,10 @@ class ManagedUIHandlerTest : public testing::Test {
         std::make_unique<policy::PolicyServiceImpl>(std::move(providers)));
     profile_ = builder.Build();
 
-    // We use a random source_name here as calling Add() can replace existing
-    // sources with the same name (which might destroy the memory addressed by
-    // |source_->GetWebUIDataSource()|.
-    content::WebUIDataSource::Add(profile(), source_->GetWebUIDataSource());
+    // `source_` has been created with a random name, so calling
+    // AddDataSourceForBrowserContext() won't accidentally destroy other data
+    // sources.
+    source_->AddDataSourceForBrowserContext(profile());
   }
 
   void TearDown() override { policy_provider()->Shutdown(); }

content/browser/webui/web_ui_data_source_impl.cc

@@ -39,22 +39,11 @@ namespace content {
 // static
 WebUIDataSource* WebUIDataSource::CreateAndAdd(BrowserContext* browser_context,
                                                const std::string& source_name) {
-  WebUIDataSource* data_source = WebUIDataSource::Create(source_name);
-  WebUIDataSource::Add(browser_context, data_source);
+  auto* data_source = new WebUIDataSourceImpl(source_name);
+  URLDataManager::AddWebUIDataSource(browser_context, data_source);
   return data_source;
 }
 
-// static
-WebUIDataSource* WebUIDataSource::Create(const std::string& source_name) {
-  return new WebUIDataSourceImpl(source_name);
-}
-
-// static
-void WebUIDataSource::Add(BrowserContext* browser_context,
-                          WebUIDataSource* source) {
-  URLDataManager::AddWebUIDataSource(browser_context, source);
-}
-
 // static
 void WebUIDataSource::Update(BrowserContext* browser_context,
                              const std::string& source_name,

content/browser/webui/web_ui_data_source_unittest.cc

@@ -83,11 +83,9 @@ class WebUIDataSourceTest : public testing::Test {
  private:
   void SetUp() override {
     SetContentClient(&client_);
-    WebUIDataSource* source = WebUIDataSourceImpl::Create("host");
-    WebUIDataSourceImpl* source_impl =
-        static_cast<WebUIDataSourceImpl*>(source);
-    source_impl->disable_load_time_data_defaults_for_testing();
-    source_ = base::WrapRefCounted(source_impl);
+    WebUIDataSourceImpl* source = new WebUIDataSourceImpl("host");
+    source->disable_load_time_data_defaults_for_testing();
+    source_ = base::WrapRefCounted(source);
   }
 
   BrowserTaskEnvironment task_environment_;

content/public/browser/web_ui_data_source.h

@@ -38,24 +38,12 @@ class WebUIDataSource {
  public:
   virtual ~WebUIDataSource() {}
 
-  // Calls `Create()` and `Add()` to internalize ownership of the
-  // WebUIDataSource instance. Callers just get a raw pointer, which they don't
-  // own. Prefer `CreateAndAdd` in new code.
+  // Creates a WebUIDataSource and adds it to the BrowserContext, which owns it.
+  // Callers just get a raw pointer, which they don't own.
   CONTENT_EXPORT static WebUIDataSource* CreateAndAdd(
       BrowserContext* browser_context,
       const std::string& source_name);
 
-  // Creates a WebUIDataSource instance. Caller takes ownership of returned
-  // pointer. Prefer `CreateAndAdd()` when possible.
-  CONTENT_EXPORT static WebUIDataSource* Create(const std::string& source_name);
-
-  // Adds a WebUI data source to |browser_context|. TODO(dbeam): update this API
-  // to take a std::unique_ptr instead to make it clear that |source| can be
-  // destroyed and references should not be kept by callers. Use |Update()|
-  // if you need to change an existing data source.
-  CONTENT_EXPORT static void Add(BrowserContext* browser_context,
-                                 WebUIDataSource* source);
-
   CONTENT_EXPORT static void Update(BrowserContext* browser_context,
                                     const std::string& source_name,
                                     const base::Value::Dict& update);

content/public/test/test_web_ui_data_source.cc

@@ -55,6 +55,10 @@ class TestWebUIDataSourceImpl : public TestWebUIDataSource {
 
   WebUIDataSource* GetWebUIDataSource() override { return source_.get(); }
 
+  void AddDataSourceForBrowserContext(BrowserContext* context) override {
+    URLDataManager::AddWebUIDataSource(context, GetWebUIDataSource());
+  }
+
  private:
   scoped_refptr<WebUIDataSourceImplWithPublicData> source_;
 };

content/public/test/test_web_ui_data_source.h

@@ -15,6 +15,7 @@ class GURL;
 
 namespace content {
 
+class BrowserContext;
 class WebUIDataSource;
 
 class TestWebUIDataSource {
@@ -31,6 +32,8 @@ class TestWebUIDataSource {
   virtual int URLToIdrOrDefault(const GURL& url) = 0;
 
   virtual WebUIDataSource* GetWebUIDataSource() = 0;
+
+  virtual void AddDataSourceForBrowserContext(BrowserContext* context) = 0;
 };
 
 }  // namespace content