Commit 910e0efa authored by Kurt Horimoto's avatar Kurt Horimoto Committed by Commit Bot

[iOS] Check against |presented_request_| when capabilities change.

When an OverlayPresentationContext's capabilities are updated,
OverlayPresenter will hide the currently-presented overlay UI if the
new capabilities are insufficient to present it.  However, the previous
implementation mistakenly checked against GetActiveRequest() rather
than |presented_request_| to handle this logic.  GetActiveRequest()
returns the front request of the active WebState's queue, which is
often, but not always the presented request when |presenting_| is true.
|presented_request_| is more accurate, as there are two points in time
where it differs from GetActiveRequest():
- When presented overlay UI is being dismissed due to request
  cancellation.  At this point, the request will have already been
  removed from the active WebState's queue.
- When presented overlay UI is being dismissed due to a change in the
  active WebState.  The request still belongs to the previously-
  active WebState, so will not be returned by GetActiveRequest().

Bug: 1065564
Change-Id: I4c7eec8ba28311f520cff482d316bb4ac8e24a23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212520Reviewed-by: default avatarChris Lu <thegreenfrog@chromium.org>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Auto-Submit: Kurt Horimoto <kkhorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#772276}
parent 4659bddd
...@@ -399,10 +399,10 @@ void OverlayPresenterImpl:: ...@@ -399,10 +399,10 @@ void OverlayPresenterImpl::
DCHECK_EQ(presentation_context_, presentation_context); DCHECK_EQ(presentation_context_, presentation_context);
// Hide the presented overlay UI if the presentation context is transitioning // Hide the presented overlay UI if the presentation context is transitioning
// to a state where that UI is not supported. // to a state where that UI is not supported.
OverlayRequest* request = GetActiveRequest(); if (presented_request_ && !presentation_context->CanShowUIForRequest(
if (request && presenting_ && presented_request_, capabilities)) {
!presentation_context->CanShowUIForRequest(request, capabilities)) { DCHECK(presenting_);
presentation_context_->HideOverlayUI(GetActiveRequest()); presentation_context_->HideOverlayUI(presented_request_);
} }
} }
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "ios/chrome/browser/overlays/public/overlay_presenter_observer.h" #include "ios/chrome/browser/overlays/public/overlay_presenter_observer.h"
#include "ios/chrome/browser/overlays/public/overlay_request.h" #include "ios/chrome/browser/overlays/public/overlay_request.h"
#include "ios/chrome/browser/overlays/test/fake_overlay_presentation_context.h" #include "ios/chrome/browser/overlays/test/fake_overlay_presentation_context.h"
#include "ios/chrome/browser/overlays/test/fake_overlay_request_cancel_handler.h"
#include "ios/chrome/browser/overlays/test/fake_overlay_user_data.h" #include "ios/chrome/browser/overlays/test/fake_overlay_user_data.h"
#import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h" #import "ios/chrome/browser/web_state_list/fake_web_state_list_delegate.h"
#import "ios/chrome/browser/web_state_list/web_state_list.h" #import "ios/chrome/browser/web_state_list/web_state_list.h"
...@@ -448,6 +449,79 @@ TEST_F(OverlayPresenterImplTest, CancelRequests) { ...@@ -448,6 +449,79 @@ TEST_F(OverlayPresenterImplTest, CancelRequests) {
presentation_context().GetPresentationState(queued_request)); presentation_context().GetPresentationState(queued_request));
} }
// Tests that the presented UI is correctly hidden if the presentation
// capabilities change to kNone during the dismissal of overlay UI whose request
// is owned by an inactive WebState.
TEST_F(OverlayPresenterImplTest,
ChangePresentationCapabilitiesDuringDismissalForInactiveWebState) {
// Insert an activated WebState to the list and add a request.
presenter().SetPresentationContext(&presentation_context());
web_state_list().InsertWebState(0, std::make_unique<web::TestWebState>(),
WebStateList::InsertionFlags::INSERT_ACTIVATE,
WebStateOpener());
OverlayRequest* request = AddRequest(active_web_state());
// Disable dismissal callbacks in the fake context and activate a new
// WebState.
presentation_context().SetDismissalCallbacksEnabled(false);
web_state_list().InsertWebState(1, std::make_unique<web::TestWebState>(),
WebStateList::InsertionFlags::INSERT_ACTIVATE,
WebStateOpener());
// Reset the presentation capabilities to kNone. Since the context can no
// longer support presenting |request|, it will be hidden.
presentation_context().SetPresentationCapabilities(
OverlayPresentationContext::UIPresentationCapabilities::kNone);
// Re-enable dismissal callbacks to dimulate the dismissal finishing after the
// presentation capability reset.
EXPECT_CALL(observer(), DidHideOverlay(&presenter(), request));
presentation_context().SetDismissalCallbacksEnabled(true);
EXPECT_EQ(FakeOverlayPresentationContext::PresentationState::kHidden,
presentation_context().GetPresentationState(request));
}
// Tests that the presented UI is correctly hidden if the presentation
// capabilities change to kNone during the dismissal of overlay UI whose request
// that was cancelled.
TEST_F(OverlayPresenterImplTest,
ChangePresentationCapabilitiesDuringDismissalForCancelledRequest) {
// Insert an activated WebState to the list.
presenter().SetPresentationContext(&presentation_context());
web_state_list().InsertWebState(0, std::make_unique<web::TestWebState>(),
WebStateList::InsertionFlags::INSERT_ACTIVATE,
WebStateOpener());
OverlayRequestQueueImpl* queue = GetQueueForWebState(active_web_state());
// Add a request with a fake cancel handler.
std::unique_ptr<OverlayRequest> inserted_request =
OverlayRequest::CreateWithConfig<FakeOverlayUserData>();
OverlayRequest* request = inserted_request.get();
std::unique_ptr<FakeOverlayRequestCancelHandler> inserted_cancel_handler =
std::make_unique<FakeOverlayRequestCancelHandler>(request, queue);
FakeOverlayRequestCancelHandler* cancel_handler =
inserted_cancel_handler.get();
EXPECT_CALL(observer(), WillShowOverlay(&presenter(), request));
queue->AddRequest(std::move(inserted_request));
// Disable dismissal callbacks in the fake context and cancel the request.
presentation_context().SetDismissalCallbacksEnabled(false);
cancel_handler->TriggerCancellation();
EXPECT_FALSE(queue->front_request());
// Reset the presentation capabilities to kNone. Since the context can no
// longer support presenting |request|, it will be hidden.
presentation_context().SetPresentationCapabilities(
OverlayPresentationContext::UIPresentationCapabilities::kNone);
// Re-enable dismissal callbacks to dimulate the dismissal finishing after the
// presentation capability reset.
EXPECT_CALL(observer(), DidHideOverlay(&presenter(), request));
presentation_context().SetDismissalCallbacksEnabled(true);
EXPECT_EQ(FakeOverlayPresentationContext::PresentationState::kHidden,
presentation_context().GetPresentationState(request));
}
// Tests that deleting the presenter // Tests that deleting the presenter
TEST_F(OverlayPresenterImplTest, PresenterWasDestroyed) { TEST_F(OverlayPresenterImplTest, PresenterWasDestroyed) {
DeleteBrowser(); DeleteBrowser();
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment