[iOS] Add OverlaySupport specification to OverlayDispatchCallbacks

OverlayDispatchCallbacks were previously a typedef of
base::RepeatingCalback that was stored in
OverlayDispatchCallbackStorage, which used a templatized API in order
to ensure that the provided callbacks were only ever executed with a
specific type of OverlayResponse.  This added additional complexity
that could not be hidden because C++ class templates need to be defined
entirely within header files since they are not linked.

This CL instead defines an OverlayDispatchCallback class that allows
for the specification of a subset of allowed OverlayResponses, removing
the need for a templatized storage class.

Bug: none
Change-Id: Ia7bbc2dc90ebf8941e12126104b076a98a6feffa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1984177
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728107}

ios/chrome/browser/overlays/BUILD.gn

@@ -6,7 +6,7 @@ source_set("overlays") {
   public = [
     "public/overlay_callback_manager.h",
     "public/overlay_dismissal_callback.h",
-    "public/overlay_dispatch_callback_storage.h",
+    "public/overlay_dispatch_callback.h",
     "public/overlay_modality.h",
     "public/overlay_presentation_callback.h",
     "public/overlay_presentation_context.h",
@@ -29,7 +29,7 @@ source_set("overlays") {
     "default_overlay_request_cancel_handler.mm",
     "overlay_callback_manager_impl.cc",
     "overlay_callback_manager_impl.h",
-    "overlay_dispatch_callback_storage.cc",
+    "overlay_dispatch_callback.cc",
     "overlay_presenter_impl.h",
     "overlay_presenter_impl.mm",
     "overlay_presenter_observer.cc",
@@ -62,6 +62,7 @@ source_set("unit_tests") {
   sources = [
     "default_overlay_request_cancel_handler_unittest.mm",
     "overlay_callback_manager_impl_unittest.cc",
+    "overlay_dispatch_callback_unittest.cc",
     "overlay_presenter_impl_unittest.mm",
     "overlay_presenter_observer_bridge_unittest.mm",
     "overlay_request_impl_unittest.cc",

ios/chrome/browser/overlays/overlay_callback_manager_impl.cc

@@ -39,10 +39,12 @@ void OverlayCallbackManagerImpl::AddCompletionCallback(
 void OverlayCallbackManagerImpl::DispatchResponse(
     std::unique_ptr<OverlayResponse> response) {
   DCHECK(response);
-  dispatch_callback_storage_.DispatchResponse(response.get());
+  for (auto& callback : dispatch_callbacks_) {
+    callback.Run(response.get());
+  }
 }
 
-OverlayDispatchCallbackStorage*
-OverlayCallbackManagerImpl::GetDispatchCallbackStorage() {
-  return &dispatch_callback_storage_;
+void OverlayCallbackManagerImpl::AddDispatchCallback(
+    OverlayDispatchCallback callback) {
+  dispatch_callbacks_.emplace_back(std::move(callback));
 }

ios/chrome/browser/overlays/overlay_callback_manager_impl.h

@@ -8,7 +8,6 @@
 #include <vector>
 
 #include "ios/chrome/browser/overlays/public/overlay_callback_manager.h"
-#include "ios/chrome/browser/overlays/public/overlay_dispatch_callback_storage.h"
 #include "ios/chrome/browser/overlays/public/overlay_response.h"
 
 // Implementation of OverlayCallbackManager.
@@ -27,14 +26,12 @@ class OverlayCallbackManagerImpl : public OverlayCallbackManager {
   OverlayResponse* GetCompletionResponse() const override;
   void AddCompletionCallback(OverlayCompletionCallback callback) override;
   void DispatchResponse(std::unique_ptr<OverlayResponse> response) override;
-  OverlayDispatchCallbackStorage* GetDispatchCallbackStorage() override;
+  void AddDispatchCallback(OverlayDispatchCallback callback) override;
 
  private:
-  // The completion response and callbacks.
   std::unique_ptr<OverlayResponse> completion_response_;
   std::vector<OverlayCompletionCallback> completion_callbacks_;
-  // The storage holding OverlayDispatchCallbacks.
-  OverlayDispatchCallbackStorage dispatch_callback_storage_;
+  std::vector<OverlayDispatchCallback> dispatch_callbacks_;
 };
 
 #endif  // IOS_CHROME_BROWSER_OVERLAYS_OVERLAY_CALLBACK_MANAGER_IMPL_H_

ios/chrome/browser/overlays/overlay_callback_manager_impl_unittest.cc

@@ -67,10 +67,12 @@ TEST_F(OverlayCallbackManagerImplTest, DispatchCallbacks) {
       ^(OverlayResponse* response) {
         ++second_execution_count;
       };
-  manager.AddDispatchCallback<FirstResponseInfo>(
-      base::BindRepeating(base::RetainBlock(first_callback_block)));
-  manager.AddDispatchCallback<SecondResponseInfo>(
-      base::BindRepeating(base::RetainBlock(second_callback_block)));
+  manager.AddDispatchCallback(OverlayDispatchCallback(
+      base::BindRepeating(base::RetainBlock(first_callback_block)),
+      FirstResponseInfo::ResponseSupport()));
+  manager.AddDispatchCallback(OverlayDispatchCallback(
+      base::BindRepeating(base::RetainBlock(second_callback_block)),
+      SecondResponseInfo::ResponseSupport()));
   ASSERT_EQ(0U, first_execution_count);
   ASSERT_EQ(0U, second_execution_count);
 

ios/chrome/browser/overlays/overlay_dispatch_callback.cc

+// Copyright 2020 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 "ios/chrome/browser/overlays/public/overlay_dispatch_callback.h"
+
+#include "base/logging.h"
+#include "ios/chrome/browser/overlays/public/overlay_response_support.h"
+
+OverlayDispatchCallback::OverlayDispatchCallback(
+    base::RepeatingCallback<void(OverlayResponse* response)> callback,
+    const OverlayResponseSupport* support)
+    : callback_(std::move(callback)), response_support_(support) {
+  DCHECK(!callback_.is_null());
+  DCHECK(response_support_);
+}
+
+OverlayDispatchCallback::OverlayDispatchCallback(
+    OverlayDispatchCallback&& other)
+    : callback_(std::move(other.callback_)),
+      response_support_(other.response_support_) {}
+
+OverlayDispatchCallback::~OverlayDispatchCallback() = default;
+
+void OverlayDispatchCallback::Run(OverlayResponse* response) {
+  if (response_support_->IsResponseSupported(response))
+    callback_.Run(response);
+}

ios/chrome/browser/overlays/overlay_dispatch_callback_storage.cc

-// Copyright 2019 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 "ios/chrome/browser/overlays/public/overlay_dispatch_callback_storage.h"
-
-#pragma mark - OverlayDispatchCallbackStorage
-
-OverlayDispatchCallbackStorage::OverlayDispatchCallbackStorage() = default;
-
-OverlayDispatchCallbackStorage::~OverlayDispatchCallbackStorage() = default;
-
-void OverlayDispatchCallbackStorage::DispatchResponse(
-    OverlayResponse* response) {
-  for (auto& list_pair : callback_lists_) {
-    list_pair.second->ExecuteCallbacks(response);
-  }
-}
-
-#pragma mark - OverlayDispatchCallbackStorage::CallbackList
-
-OverlayDispatchCallbackStorage::CallbackList::CallbackList() = default;
-
-OverlayDispatchCallbackStorage::CallbackList::~CallbackList() = default;
-
-void OverlayDispatchCallbackStorage::CallbackList::AddCallback(
-    OverlayDispatchCallback callback) {
-  DCHECK(!callback.is_null());
-  callbacks_.emplace_back(std::move(callback));
-}
-
-void OverlayDispatchCallbackStorage::CallbackList::ExecuteCallbacks(
-    OverlayResponse* response) {
-  if (!ShouldExecuteForResponse(response))
-    return;
-  for (auto& callback : callbacks_) {
-    callback.Run(response);
-  }
-}

ios/chrome/browser/overlays/overlay_dispatch_callback_unittest.cc

+// Copyright 2020 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 "ios/chrome/browser/overlays/public/overlay_dispatch_callback.h"
+
+#include "base/bind.h"
+#include "ios/chrome/browser/overlays/public/overlay_response.h"
+#include "ios/chrome/browser/overlays/test/overlay_test_macros.h"
+#include "testing/platform_test.h"
+
+namespace {
+// Response info types used in tests.
+DEFINE_TEST_OVERLAY_RESPONSE_INFO(FirstInfo);
+DEFINE_TEST_OVERLAY_RESPONSE_INFO(SecondInfo);
+}  // namespace
+
+// Test fixture for OverlayDispatchCallback.
+class OverlayDispatchCallbackTest : public PlatformTest {
+ public:
+  // Test function to be used as a dispatch callback.  Counts number of times
+  // function was called and exposes that count via execution_count().
+  void TestDispatchCallback(OverlayResponse* response) { ++execution_count_; }
+
+  // Returns the number of times TestCompletionCallback() has been executed.
+  size_t execution_count() const { return execution_count_; }
+
+ private:
+  size_t execution_count_ = 0;
+};
+
+// Tests that the OverlayDispatchCallbacks constructed with a specified
+// OverlaySupport is executed when run with a supported response type.
+TEST_F(OverlayDispatchCallbackTest, SupportedResponse) {
+  OverlayDispatchCallback callback(
+      base::BindRepeating(&OverlayDispatchCallbackTest::TestDispatchCallback,
+                          base::Unretained(this)),
+      FirstInfo::ResponseSupport());
+  std::unique_ptr<OverlayResponse> supported_response =
+      OverlayResponse::CreateWithInfo<FirstInfo>();
+  callback.Run(supported_response.get());
+  callback.Run(supported_response.get());
+  EXPECT_EQ(2U, execution_count());
+}
+
+// Tests that the OverlayDispatchCallbacks constructed with a specified
+// OverlaySupport no-ops when run with an unsupported response type.
+TEST_F(OverlayDispatchCallbackTest, UnsupportedResponse) {
+  OverlayDispatchCallback callback(
+      base::BindRepeating(&OverlayDispatchCallbackTest::TestDispatchCallback,
+                          base::Unretained(this)),
+      FirstInfo::ResponseSupport());
+  std::unique_ptr<OverlayResponse> unsupported_response =
+      OverlayResponse::CreateWithInfo<SecondInfo>();
+  callback.Run(unsupported_response.get());
+  callback.Run(unsupported_response.get());
+  EXPECT_EQ(0U, execution_count());
+}

ios/chrome/browser/overlays/public/overlay_callback_manager.h

@@ -7,12 +7,10 @@
 
 #include <memory>
 
-#include "base/callback.h"
-#include "ios/chrome/browser/overlays/public/overlay_dispatch_callback_storage.h"
+#include "ios/chrome/browser/overlays/public/overlay_dispatch_callback.h"
 #include "ios/chrome/browser/overlays/public/overlay_user_data.h"
 
 class OverlayResponse;
-
 // Completion callback for OverlayRequests.  If an overlay requires a completion
 // block to be executed after its UI is dismissed, OverlayPresenter clients can
 // provide a callback that uses the OverlayResponse provided to the request.
@@ -46,18 +44,10 @@ class OverlayCallbackManager {
   // requester for ongoing overlay UI.
   virtual void DispatchResponse(std::unique_ptr<OverlayResponse> response) = 0;
 
-  // Adds |callback| to be executed for dispatched responses with InfoType.  The
-  // provided callbacks are not guaranteed to be called, as there is no
-  // guarantee that an InfoType response will be sent for the overlay.
-  template <class InfoType>
-  void AddDispatchCallback(OverlayDispatchCallback callback) {
-    DCHECK(!callback.is_null());
-    GetDispatchCallbackStorage()->AddDispatchCallback<InfoType>(callback);
-  }
-
- protected:
-  // Returns the dispatch callback storage.
-  virtual OverlayDispatchCallbackStorage* GetDispatchCallbackStorage() = 0;
+  // Adds |callback| to be executed for dispatched responses.  The provided
+  // callbacks are not guaranteed to be called, as there is no guarantee that a
+  // supported response will be sent for the overlay.
+  virtual void AddDispatchCallback(OverlayDispatchCallback callback) = 0;
 };
 
 #endif  // IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_CALLBACK_MANAGER_H_

ios/chrome/browser/overlays/public/overlay_dispatch_callback.h

+// Copyright 2020 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.
+
+#ifndef IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_DISPATCH_CALLBACK_H_
+#define IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_DISPATCH_CALLBACK_H_
+
+#include "base/callback.h"
+
+class OverlayResponseSupport;
+class OverlayResponse;
+
+// Callback for OverlayResponses dispatched for user interaction events
+// occurring in an ongoing overlay.
+class OverlayDispatchCallback {
+ public:
+  // Constructor for a dispatch callback that executes |callback| with
+  // OverlayResponses that are supported by |support|.  |callback| and |support|
+  // must be non-null.
+  OverlayDispatchCallback(
+      base::RepeatingCallback<void(OverlayResponse* response)> callback,
+      const OverlayResponseSupport* support);
+  OverlayDispatchCallback(OverlayDispatchCallback&& other);
+  ~OverlayDispatchCallback();
+
+  // Runs |callback_| with |response| iff the response is supported by
+  // |request_support_|.
+  void Run(OverlayResponse* response);
+
+ private:
+  // The callback to be executed.
+  base::RepeatingCallback<void(OverlayResponse* response)> callback_;
+  // The OverlayResponseSupport determining which dispatch responses can be
+  // handled by the callback.
+  const OverlayResponseSupport* response_support_ = nullptr;
+};
+
+#endif  // IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_DISPATCH_CALLBACK_H_

ios/chrome/browser/overlays/public/overlay_dispatch_callback_storage.h

-// Copyright 2019 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.
-
-#ifndef IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_DISPATCH_CALLBACK_STORAGE_H_
-#define IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_DISPATCH_CALLBACK_STORAGE_H_
-
-#include <map>
-#include <vector>
-
-#include "base/callback.h"
-#include "base/logging.h"
-#include "ios/chrome/browser/overlays/public/overlay_response.h"
-
-// Callback for OverlayResponses dispatched for user interaction events
-// occurring in an ongoing overlay.
-typedef base::RepeatingCallback<void(OverlayResponse* response)>
-    OverlayDispatchCallback;
-
-// Storage object used to hold OverlayDispatchCallbacks and execute them for
-// dispatched responses.
-class OverlayDispatchCallbackStorage {
- public:
-  OverlayDispatchCallbackStorage();
-  ~OverlayDispatchCallbackStorage();
-
-  // Adds |callback| to the storage to be executed whenever DispatchResponse()
-  // is called with an OverlayResponse created with InfoType.
-  template <class InfoType>
-  void AddDispatchCallback(OverlayDispatchCallback callback) {
-    DCHECK(!callback.is_null());
-    GetCallbackList<InfoType>()->AddCallback(std::move(callback));
-  }
-
-  // Executes the added callbacks for |response|.
-  void DispatchResponse(OverlayResponse* response);
-
- protected:
-  // Helper object that stores OverlayDispatchCallbacks for OverlayResponses
-  // created with a specific info type.
-  class CallbackList {
-   public:
-    CallbackList();
-    virtual ~CallbackList();
-
-    // Adds the callback to the list.
-    void AddCallback(OverlayDispatchCallback callback);
-    // Executes the callbacks with |response| if it's supported.
-    void ExecuteCallbacks(OverlayResponse* response);
-
-   protected:
-    // Returns whether the callbacks in the list have been registered for
-    // |response|'s info type.
-    virtual bool ShouldExecuteForResponse(OverlayResponse* response) = 0;
-
-    // The callbacks stored in this list.
-    std::vector<OverlayDispatchCallback> callbacks_;
-  };
-
-  // Template used to create CallbackList instantiations for a specific info
-  // type.
-  template <class InfoType>
-  class CallbackListImpl : public CallbackList {
-   public:
-    CallbackListImpl() = default;
-    ~CallbackListImpl() override = default;
-
-   private:
-    // CallbackList:
-    bool ShouldExecuteForResponse(OverlayResponse* response) override {
-      return !!response->GetInfo<InfoType>();
-    }
-  };
-
-  // Returns the CallbackList for OverlayRequests created with InfoType,
-  // creating one if necessary.
-  template <class InfoType>
-  CallbackList* GetCallbackList() {
-    auto& callback_list = callback_lists_[InfoType::UserDataKey()];
-    if (!callback_list)
-      callback_list = std::make_unique<CallbackListImpl<InfoType>>();
-    return callback_list.get();
-  }
-
-  // Map storing the CallbackList under the user data key for each supported
-  // OverlayRequest info type.
-  std::map<const void*, std::unique_ptr<CallbackList>> callback_lists_;
-};
-
-#endif  // IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_DISPATCH_CALLBACK_STORAGE_H_