[iOS] Allow custom OverlayRequest cancellation behavior.

This CL adds OverlayRequestCancelHandler, an object supplied to the
OverlayRequestQueue that can cancel an individual request.  A default
option is available that cancels requests for all committed, document-
changing navigations.  Other overlays that may want to cancel using
different heuristics can supply a custom cancel handler subclass.

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

ios/chrome/browser/overlays/BUILD.gn

@@ -13,14 +13,18 @@ source_set("overlays") {
     "public/overlay_presenter_observer.h",
     "public/overlay_presenter_observer_bridge.h",
     "public/overlay_request.h",
+    "public/overlay_request_cancel_handler.h",
     "public/overlay_request_queue.h",
     "public/overlay_response.h",
     "public/overlay_user_data.h",
   ]
   sources = [
+    "default_overlay_request_cancel_handler.h",
+    "default_overlay_request_cancel_handler.mm",
     "overlay_presenter_impl.h",
     "overlay_presenter_impl.mm",
     "overlay_presenter_observer_bridge.mm",
+    "overlay_request_cancel_handler.mm",
     "overlay_request_impl.cc",
     "overlay_request_impl.h",
     "overlay_request_queue_impl.h",
@@ -44,6 +48,7 @@ source_set("overlays") {
 source_set("unit_tests") {
   testonly = true
   sources = [
+    "default_overlay_request_cancel_handler_unittest.mm",
     "overlay_presenter_impl_unittest.mm",
     "overlay_presenter_observer_bridge_unittest.mm",
     "overlay_request_impl_unittest.cc",

ios/chrome/browser/overlays/default_overlay_request_cancel_handler.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_DEFAULT_OVERLAY_REQUEST_CANCEL_HANDLER_H_
+#define IOS_CHROME_BROWSER_OVERLAYS_DEFAULT_OVERLAY_REQUEST_CANCEL_HANDLER_H_
+
+#include "base/scoped_observer.h"
+#import "ios/chrome/browser/overlays/public/overlay_request_cancel_handler.h"
+#import "ios/web/public/web_state.h"
+#include "ios/web/public/web_state_observer.h"
+
+// A default implementation of OverlayRequestCancelHandler.  Cancels the request
+// for committed, document-changing navigations.
+class DefaultOverlayRequestCancelHandler : public OverlayRequestCancelHandler {
+ public:
+  DefaultOverlayRequestCancelHandler(OverlayRequest* request,
+                                     OverlayRequestQueue* queue,
+                                     web::WebState* web_state);
+  ~DefaultOverlayRequestCancelHandler() override;
+
+ private:
+  // Cancels the request for navigation events.
+  void Cancel();
+
+  // Helper object that intercepts navigation events to trigger cancellation.
+  class NavigationHelper : web::WebStateObserver {
+   public:
+    NavigationHelper(DefaultOverlayRequestCancelHandler* cancel_handler,
+                     web::WebState* web_state);
+    ~NavigationHelper() override;
+
+    // web::WebStateObserver:
+    void DidFinishNavigation(
+        web::WebState* web_state,
+        web::NavigationContext* navigation_context) override;
+    void RenderProcessGone(web::WebState* web_state) override;
+    void WebStateDestroyed(web::WebState* web_state) override;
+
+   private:
+    DefaultOverlayRequestCancelHandler* cancel_handler_ = nullptr;
+    ScopedObserver<web::WebState, web::WebStateObserver> scoped_observer_;
+  };
+
+  NavigationHelper navigation_helper_;
+};
+
+#endif  // IOS_CHROME_BROWSER_OVERLAYS_DEFAULT_OVERLAY_REQUEST_CANCEL_HANDLER_H_

ios/chrome/browser/overlays/default_overlay_request_cancel_handler.mm

+// 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.
+
+#import "ios/chrome/browser/overlays/default_overlay_request_cancel_handler.h"
+
+#include "base/logging.h"
+#import "ios/web/public/navigation/navigation_context.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+DefaultOverlayRequestCancelHandler::DefaultOverlayRequestCancelHandler(
+    OverlayRequest* request,
+    OverlayRequestQueue* queue,
+    web::WebState* web_state)
+    : OverlayRequestCancelHandler(request, queue),
+      navigation_helper_(this, web_state) {}
+
+DefaultOverlayRequestCancelHandler::~DefaultOverlayRequestCancelHandler() =
+    default;
+
+void DefaultOverlayRequestCancelHandler::Cancel() {
+  CancelRequest();
+}
+
+DefaultOverlayRequestCancelHandler::NavigationHelper::NavigationHelper(
+    DefaultOverlayRequestCancelHandler* cancel_handler,
+    web::WebState* web_state)
+    : cancel_handler_(cancel_handler), scoped_observer_(this) {
+  DCHECK(cancel_handler);
+  DCHECK(web_state);
+  scoped_observer_.Add(web_state);
+}
+
+DefaultOverlayRequestCancelHandler::NavigationHelper::~NavigationHelper() =
+    default;
+
+void DefaultOverlayRequestCancelHandler::NavigationHelper::DidFinishNavigation(
+    web::WebState* web_state,
+    web::NavigationContext* navigation_context) {
+  if (navigation_context->HasCommitted() &&
+      !navigation_context->IsSameDocument()) {
+    cancel_handler_->Cancel();
+  }
+}
+
+void DefaultOverlayRequestCancelHandler::NavigationHelper::RenderProcessGone(
+    web::WebState* web_state) {
+  cancel_handler_->Cancel();
+}
+
+void DefaultOverlayRequestCancelHandler::NavigationHelper::WebStateDestroyed(
+    web::WebState* web_state) {
+  cancel_handler_->Cancel();
+  scoped_observer_.RemoveAll();
+}

ios/chrome/browser/overlays/default_overlay_request_cancel_handler_unittest.mm

+// 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.
+
+#import "ios/chrome/browser/overlays/default_overlay_request_cancel_handler.h"
+
+#include "ios/chrome/browser/overlays/public/overlay_request.h"
+#include "ios/chrome/browser/overlays/public/overlay_request_queue.h"
+#include "ios/chrome/browser/overlays/test/fake_overlay_user_data.h"
+#import "ios/web/public/test/fakes/fake_navigation_context.h"
+#import "ios/web/public/test/fakes/test_web_state.h"
+#include "testing/gmock/include/gmock/gmock.h"
+#include "testing/platform_test.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+// Test fixture for DefaultOverlayRequestCancelHandler.
+class DefaultOverlayRequestCancelHandlerTest : public PlatformTest {
+ public:
+  DefaultOverlayRequestCancelHandlerTest() : PlatformTest() {
+    std::unique_ptr<OverlayRequest> request =
+        OverlayRequest::CreateWithConfig<FakeOverlayUserData>(nullptr);
+    queue()->AddRequest(std::move(request));
+  }
+
+  OverlayRequestQueue* queue() {
+    return OverlayRequestQueue::FromWebState(&web_state_,
+                                             OverlayModality::kWebContentArea);
+  }
+  web::TestWebState* web_state() { return &web_state_; }
+
+ private:
+  web::TestWebState web_state_;
+};
+
+// Tests that the request is removed from the queue for committed, document-
+// changing navigations.
+TEST_F(DefaultOverlayRequestCancelHandlerTest, CancelForNavigations) {
+  // Simulate a navigation.
+  web::FakeNavigationContext context;
+  context.SetHasCommitted(true);
+  context.SetIsSameDocument(false);
+  web_state()->OnNavigationFinished(&context);
+
+  // Verify that the queue is empty.
+  EXPECT_FALSE(queue()->front_request());
+}
+
+// Tests that the request is removed from the queue for renderer crashes.
+TEST_F(DefaultOverlayRequestCancelHandlerTest, CancelForRendererCrashes) {
+  web_state()->OnRenderProcessGone();
+
+  // Verify that the queue is empty.
+  EXPECT_FALSE(queue()->front_request());
+}

ios/chrome/browser/overlays/overlay_request_cancel_handler.mm

+// 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.
+
+#import "ios/chrome/browser/overlays/public/overlay_request_cancel_handler.h"
+
+#include "base/logging.h"
+#import "ios/chrome/browser/overlays/public/overlay_request_queue.h"
+
+#if !defined(__has_feature) || !__has_feature(objc_arc)
+#error "This file requires ARC support."
+#endif
+
+OverlayRequestCancelHandler::OverlayRequestCancelHandler(
+    OverlayRequest* request,
+    OverlayRequestQueue* queue)
+    : request_(request), queue_(queue) {
+  DCHECK(request_);
+  DCHECK(queue_);
+}
+
+void OverlayRequestCancelHandler::CancelRequest() {
+  queue_->CancelRequest(request_);
+}

ios/chrome/browser/overlays/overlay_request_queue_impl.h

@@ -62,9 +62,9 @@ class OverlayRequestQueueImpl : public OverlayRequestQueue {
   base::WeakPtr<OverlayRequestQueueImpl> GetWeakPtr();
 
   // Whether the queue is empty.
-  bool empty() const { return requests_.empty(); }
+  bool empty() const { return request_storages_.empty(); }
   // The number of requests in the queue.
-  size_t size() const { return requests_.size(); }
+  size_t size() const { return request_storages_.size(); }
 
   // Removes the front or back request from the queue, transferring ownership of
   // the request to the caller.  Must be called on a non-empty queue.
@@ -72,36 +72,36 @@ class OverlayRequestQueueImpl : public OverlayRequestQueue {
   std::unique_ptr<OverlayRequest> PopBackRequest();
 
   // OverlayRequestQueue:
+  void AddRequest(
+      std::unique_ptr<OverlayRequest> request,
+      std::unique_ptr<OverlayRequestCancelHandler> cancel_handler) override;
   void AddRequest(std::unique_ptr<OverlayRequest> request) override;
   OverlayRequest* front_request() const override;
   void CancelAllRequests() override;
+  void CancelRequest(OverlayRequest* request) override;
 
  private:
   // Private constructor called by container.
   explicit OverlayRequestQueueImpl(web::WebState* web_state);
 
-  // Helper object that cancels requests for navigation events.
-  class RequestCancellationHelper : public web::WebStateObserver {
-   public:
-    RequestCancellationHelper(OverlayRequestQueueImpl* queue,
-                              web::WebState* web_state);
+  // Helper object that stores OverlayRequests along with their cancellation
+  // handlers.
+  struct OverlayRequestStorage {
+    OverlayRequestStorage(
+        std::unique_ptr<OverlayRequest> request,
+        std::unique_ptr<OverlayRequestCancelHandler> cancel_handler);
+    ~OverlayRequestStorage();
 
-   private:
-    // web::WebStateObserver:
-    void DidFinishNavigation(
-        web::WebState* web_state,
-        web::NavigationContext* navigation_context) override;
-    void RenderProcessGone(web::WebState* web_state) override;
-    void WebStateDestroyed(web::WebState* web_state) override;
-
-    OverlayRequestQueueImpl* queue_ = nullptr;
+    std::unique_ptr<OverlayRequest> request;
+    std::unique_ptr<OverlayRequestCancelHandler> cancel_handler;
   };
 
-  RequestCancellationHelper cancellation_helper_;
+  web::WebState* web_state_ = nullptr;
   base::ObserverList<Observer, /* check_empty= */ true> observers_;
   // The queue used to hold the received requests.  Stored as a circular dequeue
   // to allow performant pop events from the front of the queue.
-  base::circular_deque<std::unique_ptr<OverlayRequest>> requests_;
+  base::circular_deque<std::unique_ptr<OverlayRequestStorage>>
+      request_storages_;
   base::WeakPtrFactory<OverlayRequestQueueImpl> weak_factory_;
 };
 

ios/chrome/browser/overlays/overlay_request_queue_impl.mm

@@ -8,6 +8,7 @@
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#import "ios/chrome/browser/overlays/default_overlay_request_cancel_handler.h"
 #include "ios/chrome/browser/overlays/public/overlay_request.h"
 #import "ios/web/public/navigation/navigation_context.h"
 
@@ -44,7 +45,7 @@ OverlayRequestQueueImpl* OverlayRequestQueueImpl::Container::QueueForModality(
 #pragma mark - OverlayRequestQueueImpl
 
 OverlayRequestQueueImpl::OverlayRequestQueueImpl(web::WebState* web_state)
-    : cancellation_helper_(this, web_state), weak_factory_(this) {}
+    : web_state_(web_state), weak_factory_(this) {}
 OverlayRequestQueueImpl::~OverlayRequestQueueImpl() = default;
 
 #pragma mark Public
@@ -62,31 +63,46 @@ base::WeakPtr<OverlayRequestQueueImpl> OverlayRequestQueueImpl::GetWeakPtr() {
 }
 
 std::unique_ptr<OverlayRequest> OverlayRequestQueueImpl::PopFrontRequest() {
-  DCHECK(!requests_.empty());
-  std::unique_ptr<OverlayRequest> request = std::move(requests_.front());
-  requests_.pop_front();
+  DCHECK(!request_storages_.empty());
+  std::unique_ptr<OverlayRequest> request =
+      std::move(request_storages_.front()->request);
+  request_storages_.pop_front();
   return request;
 }
 
 std::unique_ptr<OverlayRequest> OverlayRequestQueueImpl::PopBackRequest() {
-  DCHECK(!requests_.empty());
-  std::unique_ptr<OverlayRequest> request = std::move(requests_.back());
-  requests_.pop_back();
+  DCHECK(!request_storages_.empty());
+  std::unique_ptr<OverlayRequest> request =
+      std::move(request_storages_.back()->request);
+  request_storages_.pop_back();
   return request;
 }
 
 #pragma mark OverlayRequestQueue
 
 void OverlayRequestQueueImpl::AddRequest(
-    std::unique_ptr<OverlayRequest> request) {
-  requests_.push_back(std::move(request));
+    std::unique_ptr<OverlayRequest> request,
+    std::unique_ptr<OverlayRequestCancelHandler> cancel_handler) {
+  DCHECK(request.get());
+  DCHECK(cancel_handler.get());
+  request_storages_.push_back(std::make_unique<OverlayRequestStorage>(
+      std::move(request), std::move(cancel_handler)));
   for (auto& observer : observers_) {
-    observer.RequestAddedToQueue(this, requests_.back().get());
+    observer.RequestAddedToQueue(this, request_storages_.back()->request.get());
   }
 }
 
+void OverlayRequestQueueImpl::AddRequest(
+    std::unique_ptr<OverlayRequest> request) {
+  std::unique_ptr<OverlayRequestCancelHandler> cancel_handler =
+      std::make_unique<DefaultOverlayRequestCancelHandler>(request.get(), this,
+                                                           web_state_);
+  AddRequest(std::move(request), std::move(cancel_handler));
+}
+
 OverlayRequest* OverlayRequestQueueImpl::front_request() const {
-  return requests_.empty() ? nullptr : requests_.front().get();
+  return request_storages_.empty() ? nullptr
+                                   : request_storages_.front()->request.get();
 }
 
 void OverlayRequestQueueImpl::CancelAllRequests() {
@@ -94,37 +110,37 @@ void OverlayRequestQueueImpl::CancelAllRequests() {
     // Requests are cancelled in reverse order to prevent attempting to present
     // subsequent requests after the dismissal of the front request's UI.
     for (auto& observer : observers_) {
-      observer.QueuedRequestCancelled(this, requests_.back().get());
+      observer.QueuedRequestCancelled(this,
+                                      request_storages_.back()->request.get());
     }
     PopBackRequest();
   }
 }
 
-#pragma mark RequestCancellationHelper
-
-OverlayRequestQueueImpl::RequestCancellationHelper::RequestCancellationHelper(
-    OverlayRequestQueueImpl* queue,
-    web::WebState* web_state)
-    : queue_(queue) {
-  web_state->AddObserver(this);
-}
+void OverlayRequestQueueImpl::CancelRequest(OverlayRequest* request) {
+  // Find the iterator for the storage holding |request|.
+  auto storage_iter = request_storages_.begin();
+  auto end = request_storages_.end();
+  while (storage_iter != end) {
+    if ((*storage_iter)->request.get() == request)
+      break;
+    ++storage_iter;
+  }
+  if (storage_iter == end)
+    return;
 
-void OverlayRequestQueueImpl::RequestCancellationHelper::DidFinishNavigation(
-    web::WebState* web_state,
-    web::NavigationContext* navigation_context) {
-  if (navigation_context->HasCommitted() &&
-      !navigation_context->IsSameDocument()) {
-    queue_->CancelAllRequests();
+  // Notify observers of cancellation and remove the storage.
+  for (auto& observer : observers_) {
+    observer.QueuedRequestCancelled(this, request);
   }
+  request_storages_.erase(storage_iter);
 }
 
-void OverlayRequestQueueImpl::RequestCancellationHelper::RenderProcessGone(
-    web::WebState* web_state) {
-  queue_->CancelAllRequests();
-}
+#pragma mark OverlayRequestStorage
 
-void OverlayRequestQueueImpl::RequestCancellationHelper::WebStateDestroyed(
-    web::WebState* web_state) {
-  queue_->CancelAllRequests();
-  web_state->RemoveObserver(this);
-}
+OverlayRequestQueueImpl::OverlayRequestStorage::OverlayRequestStorage(
+    std::unique_ptr<OverlayRequest> request,
+    std::unique_ptr<OverlayRequestCancelHandler> cancel_handler)
+    : request(std::move(request)), cancel_handler(std::move(cancel_handler)) {}
+
+OverlayRequestQueueImpl::OverlayRequestStorage::~OverlayRequestStorage() {}

ios/chrome/browser/overlays/overlay_request_queue_impl_unittest.mm

@@ -5,6 +5,7 @@
 #import "ios/chrome/browser/overlays/overlay_request_queue_impl.h"
 
 #include "ios/chrome/browser/overlays/public/overlay_request.h"
+#import "ios/chrome/browser/overlays/public/overlay_request_cancel_handler.h"
 #include "ios/chrome/browser/overlays/test/fake_overlay_user_data.h"
 #import "ios/web/public/test/fakes/test_web_state.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -27,6 +28,16 @@ class MockOverlayRequestQueueImplObserver
   MOCK_METHOD2(QueuedRequestCancelled,
                void(OverlayRequestQueueImpl*, OverlayRequest*));
 };
+
+// Custom cancel handler that can be manually triggered.
+class FakeCancelHandler : public OverlayRequestCancelHandler {
+ public:
+  FakeCancelHandler(OverlayRequest* request, OverlayRequestQueue* queue)
+      : OverlayRequestCancelHandler(request, queue) {}
+
+  // Cancels the associated request.
+  void TriggerCancellation() { CancelRequest(); }
+};
 }  // namespace
 
 // Test fixture for RequestQueueImpl.
@@ -110,3 +121,25 @@ TEST_F(OverlayRequestQueueImplTest, CancelAllRequests) {
   EXPECT_EQ(0U, queue()->size());
   EXPECT_TRUE(queue()->empty());
 }
+
+// Tests that state is updated correctly and observer callbacks are received
+// when cancelling a request with a custom cancel handler.
+TEST_F(OverlayRequestQueueImplTest, CustomCancelHandler) {
+  std::unique_ptr<OverlayRequest> passed_request =
+      OverlayRequest::CreateWithConfig<FakeOverlayUserData>(nullptr);
+  OverlayRequest* request = passed_request.get();
+  std::unique_ptr<FakeCancelHandler> passed_cancel_handler =
+      std::make_unique<FakeCancelHandler>(request, queue());
+  FakeCancelHandler* cancel_handler = passed_cancel_handler.get();
+  EXPECT_CALL(observer(), RequestAddedToQueue(queue(), request));
+  queue()->AddRequest(std::move(passed_request),
+                      std::move(passed_cancel_handler));
+
+  // Trigger cancellation via the cancel handler, and verify that the request is
+  // correctly removed.
+  EXPECT_CALL(observer(), QueuedRequestCancelled(queue(), request));
+  cancel_handler->TriggerCancellation();
+
+  EXPECT_EQ(0U, queue()->size());
+  EXPECT_TRUE(queue()->empty());
+}

ios/chrome/browser/overlays/public/overlay_request_cancel_handler.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_REQUEST_CANCEL_HANDLER_H_
+#define IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_REQUEST_CANCEL_HANDLER_H_
+
+#include <memory>
+
+class OverlayRequest;
+class OverlayRequestQueue;
+
+// Handles the cancellation of OverlayRequests added to an OverlayRequestQueue.
+class OverlayRequestCancelHandler {
+ public:
+  virtual ~OverlayRequestCancelHandler() = default;
+
+ protected:
+  // Constructor for a cancellation handler that cancels |request| from |queue|.
+  OverlayRequestCancelHandler(OverlayRequest* request,
+                              OverlayRequestQueue* queue);
+
+  // Called by subclasses to cancel the associated request.
+  void CancelRequest();
+
+ private:
+  OverlayRequest* request_ = nullptr;
+  OverlayRequestQueue* queue_ = nullptr;
+};
+
+#endif  // IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_REQUEST_CANCEL_HANDLER_H_

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

@@ -6,6 +6,7 @@
 #define IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_REQUEST_QUEUE_H_
 
 #include "ios/chrome/browser/overlays/public/overlay_modality.h"
+#import "ios/chrome/browser/overlays/public/overlay_request_cancel_handler.h"
 
 class OverlayRequest;
 namespace web {
@@ -22,7 +23,14 @@ class OverlayRequestQueue {
                                            OverlayModality modality);
 
   // Adds |request| to be displayed alongside the content area of queue's
-  // corresponding WebState.
+  // corresponding WebState.  The request may be cancelled by
+  // |cancel_handler|.
+  virtual void AddRequest(
+      std::unique_ptr<OverlayRequest> request,
+      std::unique_ptr<OverlayRequestCancelHandler> cancel_handler) = 0;
+
+  // Adds |request| to the queue using a default cancellation handler that
+  // cancels requests for committed, document-changing navigations.
   virtual void AddRequest(std::unique_ptr<OverlayRequest> request) = 0;
 
   // Returns the front request in the queue, or nullptr if the queue is empty.
@@ -37,7 +45,11 @@ class OverlayRequestQueue {
   OverlayRequestQueue() = default;
 
  private:
+  friend class OverlayRequestCancelHandler;
   DISALLOW_COPY_AND_ASSIGN(OverlayRequestQueue);
+
+  // Called by cancellation handlers to cancel |request|.
+  virtual void CancelRequest(OverlayRequest* request) = 0;
 };
 
 #endif  // IOS_CHROME_BROWSER_OVERLAYS_PUBLIC_OVERLAY_REQUEST_QUEUE_H_