Simplify ProtocolHandlerRegistry::Delegate

This CL changes RegisterWithOSAsDefaultClient() and
CheckDefaultClientWithOS() to directly receive the callback
instead of calling GetDefaultWebClientCallback() on the
registry.

Bug: 1021600
Change-Id: Ic71173e91121f0f3576b38532e030227f3f23797
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894698
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Martin Šrámek <msramek@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713514}

chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc

@@ -505,10 +505,11 @@ class FakeProtocolHandlerRegistryDelegate
 
   void RegisterWithOSAsDefaultClient(
       const std::string& protocol,
-      ProtocolHandlerRegistry* registry) override {}
+      shell_integration::DefaultWebClientWorkerCallback callback) override {}
 
-  void CheckDefaultClientWithOS(const std::string& protocol,
-                                ProtocolHandlerRegistry* registry) override {}
+  void CheckDefaultClientWithOS(
+      const std::string& protocol,
+      shell_integration::DefaultWebClientWorkerCallback callback) override {}
 
  private:
   std::set<std::string> registered_protocols_;

chrome/browser/browsing_data/counters/site_settings_counter_unittest.cc

@@ -5,6 +5,8 @@
 #include "chrome/browser/browsing_data/counters/site_settings_counter.h"
 
 #include <memory>
+#include <string>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/containers/flat_set.h"
@@ -54,18 +56,17 @@ class TestProtocolHandlerRegistryDelegate
   }
   void RegisterWithOSAsDefaultClient(
       const std::string& protocol,
-      ProtocolHandlerRegistry* registry) override {
+      shell_integration::DefaultWebClientWorkerCallback callback) override {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
-        base::BindOnce(registry->GetDefaultWebClientCallback(protocol),
-                       shell_integration::NOT_DEFAULT));
+        base::BindOnce(std::move(callback), shell_integration::NOT_DEFAULT));
   }
-  void CheckDefaultClientWithOS(const std::string& protocol,
-                                ProtocolHandlerRegistry* registry) override {
+  void CheckDefaultClientWithOS(
+      const std::string& protocol,
+      shell_integration::DefaultWebClientWorkerCallback callback) override {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
-        base::BindOnce(registry->GetDefaultWebClientCallback(protocol),
-                       shell_integration::NOT_DEFAULT));
+        base::BindOnce(std::move(callback), shell_integration::NOT_DEFAULT));
   }
 
  private:

chrome/browser/custom_handlers/protocol_handler_registry.cc

@@ -95,23 +95,24 @@ bool ProtocolHandlerRegistry::Delegate::IsExternalHandlerRegistered(
 }
 
 void ProtocolHandlerRegistry::Delegate::RegisterWithOSAsDefaultClient(
-    const std::string& protocol, ProtocolHandlerRegistry* registry) {
+    const std::string& protocol,
+    shell_integration::DefaultWebClientWorkerCallback callback) {
   // The worker pointer is reference counted. While it is running, the
   // sequence it runs on will hold references it will be automatically freed
   // once all its tasks have finished.
   base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(
-      registry->GetDefaultWebClientCallback(protocol), protocol)
+      std::move(callback), protocol)
       ->StartSetAsDefault();
 }
 
 void ProtocolHandlerRegistry::Delegate::CheckDefaultClientWithOS(
     const std::string& protocol,
-    ProtocolHandlerRegistry* registry) {
+    shell_integration::DefaultWebClientWorkerCallback callback) {
   // The worker pointer is reference counted. While it is running, the
   // sequence it runs on will hold references it will be automatically freed
   // once all its tasks have finished.
   base::MakeRefCounted<shell_integration::DefaultProtocolClientWorker>(
-      registry->GetDefaultWebClientCallback(protocol), protocol)
+      std::move(callback), protocol)
       ->StartCheckIsDefault();
 }
 
@@ -259,7 +260,9 @@ void ProtocolHandlerRegistry::InitProtocolSettings() {
   if (ShouldRemoveHandlersNotInOS()) {
     for (ProtocolHandlerMap::const_iterator p = default_handlers_.begin();
          p != default_handlers_.end(); ++p) {
-      delegate_->CheckDefaultClientWithOS(p->second.protocol(), this);
+      const std::string& protocol = p->second.protocol();
+      delegate_->CheckDefaultClientWithOS(
+          protocol, GetDefaultWebClientCallback(protocol));
     }
   }
 }
@@ -591,14 +594,17 @@ ProtocolHandlerRegistry::GetHandlerList(
 
 void ProtocolHandlerRegistry::SetDefault(const ProtocolHandler& handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  ProtocolHandlerMap::const_iterator p = default_handlers_.find(
-      handler.protocol());
+
+  const std::string& protocol = handler.protocol();
+  ProtocolHandlerMap::const_iterator p = default_handlers_.find(protocol);
   // If we're not loading, and we are setting a default for a new protocol,
   // register with the OS.
   if (!is_loading_ && p == default_handlers_.end())
-      delegate_->RegisterWithOSAsDefaultClient(handler.protocol(), this);
-  default_handlers_.erase(handler.protocol());
-  default_handlers_.insert(std::make_pair(handler.protocol(), handler));
+    delegate_->RegisterWithOSAsDefaultClient(
+        protocol, GetDefaultWebClientCallback(protocol));
+  default_handlers_.erase(protocol);
+  default_handlers_.insert(std::make_pair(protocol, handler));
+
   PromoteHandler(handler);
 }
 

chrome/browser/custom_handlers/protocol_handler_registry.h

@@ -52,9 +52,10 @@ class ProtocolHandlerRegistry : public KeyedService {
     virtual bool IsExternalHandlerRegistered(const std::string& protocol);
     virtual void RegisterWithOSAsDefaultClient(
         const std::string& protocol,
-        ProtocolHandlerRegistry* registry);
-    virtual void CheckDefaultClientWithOS(const std::string& protocol,
-                                          ProtocolHandlerRegistry* registry);
+        shell_integration::DefaultWebClientWorkerCallback callback);
+    virtual void CheckDefaultClientWithOS(
+        const std::string& protocol,
+        shell_integration::DefaultWebClientWorkerCallback callback);
   };
 
   class Observer : public base::CheckedObserver {
@@ -194,11 +195,6 @@ class ProtocolHandlerRegistry : public KeyedService {
   // load command was issued, otherwise the command will be ignored.
   void AddPredefinedHandler(const ProtocolHandler& handler);
 
-  // Gets the callback for DefaultProtocolClientWorker. Allows the Delegate to
-  // create the worker on behalf of ProtocolHandlerRegistry.
-  shell_integration::DefaultWebClientWorkerCallback GetDefaultWebClientCallback(
-      const std::string& protocol);
-
  private:
   friend class base::DeleteHelper<ProtocolHandlerRegistry>;
   friend struct content::BrowserThread::DeleteOnThread<
@@ -294,6 +290,10 @@ class ProtocolHandlerRegistry : public KeyedService {
       const std::string& protocol,
       shell_integration::DefaultWebClientState state);
 
+  // Gets the callback for DefaultProtocolClientWorker.
+  shell_integration::DefaultWebClientWorkerCallback GetDefaultWebClientCallback(
+      const std::string& protocol);
+
   // Map from protocols (strings) to protocol handlers.
   ProtocolHandlerMultiMap protocol_handlers_;
 

chrome/browser/custom_handlers/protocol_handler_registry_unittest.cc

@@ -8,6 +8,7 @@
 
 #include <memory>
 #include <set>
+#include <utility>
 
 #include "base/bind.h"
 #include "base/run_loop.h"
@@ -66,14 +67,14 @@ class FakeDelegate : public ProtocolHandlerRegistry::Delegate {
 
   void RegisterWithOSAsDefaultClient(
       const std::string& protocol,
-      ProtocolHandlerRegistry* registry) override {
+      shell_integration::DefaultWebClientWorkerCallback callback) override {
     // Do as-if the registration has to run on another sequence and post back
     // the result with a task to the current thread.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
-        base::BindOnce(registry->GetDefaultWebClientCallback(protocol),
-                       force_os_failure_ ? shell_integration::NOT_DEFAULT
-                                         : shell_integration::IS_DEFAULT));
+        base::BindOnce(callback, force_os_failure_
+                                     ? shell_integration::NOT_DEFAULT
+                                     : shell_integration::IS_DEFAULT));
 
     if (!force_os_failure_)
       os_registered_protocols_.insert(protocol);

chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc

@@ -863,7 +863,7 @@ class FakeDelegate : public ProtocolHandlerRegistry::Delegate {
 
   void RegisterWithOSAsDefaultClient(
       const std::string& protocol,
-      ProtocolHandlerRegistry* registry) override {
+      shell_integration::DefaultWebClientWorkerCallback callback) override {
     VLOG(1) << "Register With OS";
   }
 };