[iOS] Include request in OverlayPresenter::Observer::DidHideOverlay().

Observers who had started UI because of overlay presentation previously
would have had to store state to track whether the UI has been started
so that it could be stopped when the corresponding DidHideOverlay()
signal was received.  This CL updates DidHideOverlay() to include the
request corresponding with the overlay that was stopped so observers
could detect whether UI needs to be updated based off of the request
instead of storing this state.

Implementing this callback required updating OverlayRequestQueueImpl
to return unique pointers of popped requests so that the request
lifetime can be extended beyond the observer callback.

Bug: 941745
Change-Id: I256fa21766148ea4b8856c86e3d3e99517b48be4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1659232
Commit-Queue: Kurt Horimoto <kkhorimoto@chromium.org>
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Auto-Submit: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#671466}

ios/chrome/browser/overlays/overlay_presenter_impl.mm

@@ -6,6 +6,7 @@
 
 #include "base/logging.h"
 #import "ios/chrome/browser/main/browser.h"
+#include "ios/chrome/browser/overlays/public/overlay_request.h"
 #import "ios/chrome/browser/web_state_list/web_state_list.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -213,17 +214,19 @@ void OverlayPresenterImpl::OverlayWasDismissed(
   // Pop the request for overlays dismissed by the user.  The check against the
   // queue's front request prevents popping the request twice in the event that
   // the front request was cancelled by the queue during a user-triggered
-  // dismissal.
+  // dismissal.  |popped_request| is used to extend the lifetime of the request
+  // past the DidHideOverlay() callbacks.
+  std::unique_ptr<OverlayRequest> popped_request;
   if (reason == OverlayDismissalReason::kUserInteraction && queue &&
       queue->front_request() == request) {
-    queue->PopFrontRequest();
+    popped_request = queue->PopFrontRequest();
   }
 
   presenting_ = false;
 
   // Notify the observers that the overlay UI was hidden.
   for (auto& observer : observers_) {
-    observer.DidHideOverlay(this);
+    observer.DidHideOverlay(this, request);
   }
 
   // Only show the next overlay if the active request has changed, either

ios/chrome/browser/overlays/overlay_presenter_impl_unittest.mm

@@ -31,7 +31,7 @@ class MockOverlayPresenterObserver : public OverlayPresenter::Observer {
   ~MockOverlayPresenterObserver() {}
 
   MOCK_METHOD2(WillShowOverlay, void(OverlayPresenter*, OverlayRequest*));
-  MOCK_METHOD1(DidHideOverlay, void(OverlayPresenter*));
+  MOCK_METHOD2(DidHideOverlay, void(OverlayPresenter*, OverlayRequest*));
 
   // OverlayPresenter's ObserverList checks that it is empty upon deallocation,
   // so a custom verification implemetation must be created.
@@ -157,8 +157,8 @@ TEST_F(OverlayPresenterImplTest, ResetUIDelegate) {
   // Reset the UI delegate and verify that the overlay UI is cancelled in the
   // previous delegate's context and presented in the new delegate's context.
   FakeOverlayPresenterUIDelegate new_ui_delegate;
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), request));
   EXPECT_CALL(observer(), WillShowOverlay(&presenter(), request));
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
   presenter().SetUIDelegate(&new_ui_delegate);
   EXPECT_EQ(FakeOverlayPresenterUIDelegate::PresentationState::kCancelled,
             ui_delegate().GetPresentationState(request));
@@ -168,7 +168,7 @@ TEST_F(OverlayPresenterImplTest, ResetUIDelegate) {
 
   // Reset the UI delegate to nullptr and verify that the overlay UI is
   // cancelled in |new_ui_delegate|'s context.
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), request));
   presenter().SetUIDelegate(nullptr);
   EXPECT_EQ(FakeOverlayPresenterUIDelegate::PresentationState::kCancelled,
             new_ui_delegate.GetPresentationState(request));
@@ -217,7 +217,7 @@ TEST_F(OverlayPresenterImplTest, ChangeActiveWebStateWhilePresenting) {
       std::make_unique<web::TestWebState>();
   web::WebState* second_web_state = passed_web_state.get();
   OverlayRequest* second_request = AddRequest(second_web_state);
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), first_request));
   web_state_list().InsertWebState(/*index=*/1, std::move(passed_web_state),
                                   WebStateList::InsertionFlags::INSERT_ACTIVATE,
                                   WebStateOpener());
@@ -231,8 +231,8 @@ TEST_F(OverlayPresenterImplTest, ChangeActiveWebStateWhilePresenting) {
 
   // Reactivate the first WebState and verify that its overlay is presented
   // while the second WebState's overlay is hidden.
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), second_request));
   EXPECT_CALL(observer(), WillShowOverlay(&presenter(), first_request));
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
   web_state_list().ActivateWebStateAt(0);
   EXPECT_EQ(FakeOverlayPresenterUIDelegate::PresentationState::kPresented,
             ui_delegate().GetPresentationState(first_request));
@@ -257,7 +257,7 @@ TEST_F(OverlayPresenterImplTest, ReplaceActiveWebState) {
       std::make_unique<web::TestWebState>();
   web::WebState* replacement_web_state = passed_web_state.get();
   OverlayRequest* replacement_request = AddRequest(replacement_web_state);
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), first_request));
   web_state_list().ReplaceWebStateAt(/*index=*/0, std::move(passed_web_state));
 
   // Verify that the previously shown overlay is canceled and that the overlay
@@ -281,7 +281,7 @@ TEST_F(OverlayPresenterImplTest, RemoveActiveWebState) {
             ui_delegate().GetPresentationState(request));
 
   // Remove the WebState and verify that its overlay was cancelled.
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), request));
   web_state_list().CloseWebStateAt(/*index=*/0, /* close_flags= */ 0);
   EXPECT_EQ(FakeOverlayPresenterUIDelegate::PresentationState::kCancelled,
             ui_delegate().GetPresentationState(request));
@@ -306,7 +306,7 @@ TEST_F(OverlayPresenterImplTest, DismissForUserInteraction) {
 
   // Dismiss the overlay and check that the second request's overlay is
   // presented.
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), first_request));
   ui_delegate().SimulateDismissalForRequest(
       first_request, OverlayDismissalReason::kUserInteraction);
 
@@ -334,7 +334,7 @@ TEST_F(OverlayPresenterImplTest, CancelRequests) {
             ui_delegate().GetPresentationState(active_request));
 
   // Cancel the queue's requests and verify that the UI is also cancelled.
-  EXPECT_CALL(observer(), DidHideOverlay(&presenter()));
+  EXPECT_CALL(observer(), DidHideOverlay(&presenter(), active_request));
   queue->CancelAllRequests();
   EXPECT_EQ(FakeOverlayPresenterUIDelegate::PresentationState::kCancelled,
             ui_delegate().GetPresentationState(active_request));

ios/chrome/browser/overlays/overlay_request_queue_impl.h

@@ -66,10 +66,10 @@ class OverlayRequestQueueImpl : public OverlayRequestQueue {
   // The number of requests in the queue.
   size_t size() const { return requests_.size(); }
 
-  // Removes the front or back request from the queue.  Must be called on a non-
-  // empty queue.
-  void PopFrontRequest();
-  void PopBackRequest();
+  // 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.
+  std::unique_ptr<OverlayRequest> PopFrontRequest();
+  std::unique_ptr<OverlayRequest> PopBackRequest();
 
   // OverlayRequestQueue:
   void AddRequest(std::unique_ptr<OverlayRequest> request) override;

ios/chrome/browser/overlays/overlay_request_queue_impl.mm

@@ -61,14 +61,18 @@ base::WeakPtr<OverlayRequestQueueImpl> OverlayRequestQueueImpl::GetWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
-void OverlayRequestQueueImpl::PopFrontRequest() {
+std::unique_ptr<OverlayRequest> OverlayRequestQueueImpl::PopFrontRequest() {
   DCHECK(!requests_.empty());
+  std::unique_ptr<OverlayRequest> request = std::move(requests_.front());
   requests_.pop_front();
+  return request;
 }
 
-void OverlayRequestQueueImpl::PopBackRequest() {
+std::unique_ptr<OverlayRequest> OverlayRequestQueueImpl::PopBackRequest() {
   DCHECK(!requests_.empty());
+  std::unique_ptr<OverlayRequest> request = std::move(requests_.back());
   requests_.pop_back();
+  return request;
 }
 
 #pragma mark OverlayRequestQueue

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

@@ -69,8 +69,10 @@ class OverlayPresenter {
     virtual void WillShowOverlay(OverlayPresenter* presenter,
                                  OverlayRequest* request) {}
 
-    // Called when |presenter| is finished dismissing its overlay UI.
-    virtual void DidHideOverlay(OverlayPresenter* presenter) {}
+    // Called when |presenter| is finished dismissing the overlay UI for
+    // |request|.
+    virtual void DidHideOverlay(OverlayPresenter* presenter,
+                                OverlayRequest* request) {}
 
     // Called when |presenter| is destroyed.
     virtual void OverlayPresenterDestroyed(OverlayPresenter* presenter) {}