Commit 716708fc authored by Chris Lu's avatar Chris Lu Committed by Commit Bot

[ios] Null OverlayRequestCoordinator's delegate when Browser destroyed

If the Browser is destroyed, then OverlayPresentationContextImpl will
soon be as well. Thus, OverlayRequestCoordinator's reference to it
will be invalid, so OverlayPresentationContextImpl should remove
itself as the delegate.

This change also uses __weak references to self
InfobarBannerOverlayCoordaintor.

Bug: 1129962, 1129788
Change-Id: I082ad3f7acf671b61e23434f918d22d900d5227f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427407Reviewed-by: default avatarSergio Collazos <sczs@chromium.org>
Commit-Queue: Chris Lu <thegreenfrog@chromium.org>
Auto-Submit: Chris Lu <thegreenfrog@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810916}
parent 9e86d13d
......@@ -102,11 +102,16 @@
self.bannerTransitionDriver.bannerPositioner = self;
self.bannerViewController.transitioningDelegate = self.bannerTransitionDriver;
self.bannerViewController.interactionDelegate = self.bannerTransitionDriver;
[self.baseViewController presentViewController:self.viewController
animated:animated
completion:^{
[self finishPresentation];
}];
__weak InfobarBannerOverlayCoordinator* weakSelf = self;
[self.baseViewController
presentViewController:self.viewController
animated:animated
completion:^{
InfobarBannerOverlayCoordinator* strongSelf = weakSelf;
if (strongSelf) {
[strongSelf finishPresentation];
}
}];
self.started = YES;
if (!UIAccessibilityIsVoiceOverRunning()) {
......@@ -128,10 +133,16 @@
// Mark started as NO before calling dismissal callback to prevent dup
// stopAnimated: executions.
self.started = NO;
[self.baseViewController dismissViewControllerAnimated:animated
completion:^{
[self finishDismissal];
}];
__weak InfobarBannerOverlayCoordinator* weakSelf = self;
[self.baseViewController
dismissViewControllerAnimated:animated
completion:^{
InfobarBannerOverlayCoordinator* strongSelf =
weakSelf;
if (strongSelf) {
[strongSelf finishDismissal];
}
}];
}
- (UIViewController*)viewController {
......@@ -145,7 +156,9 @@
// Notify the presentation context that the presentation has finished. This
// is necessary to synchronize OverlayPresenter scheduling logic with the UI
// layer.
self.delegate->OverlayUIDidFinishPresentation(self.request);
if (self.delegate) {
self.delegate->OverlayUIDidFinishPresentation(self.request);
}
UpdateBannerAccessibilityForPresentation(self.baseViewController,
self.viewController.view);
}
......@@ -160,7 +173,9 @@
// Notify the presentation context that the dismissal has finished. This
// is necessary to synchronize OverlayPresenter scheduling logic with the UI
// layer.
self.delegate->OverlayUIDidFinishDismissal(self.request);
if (self.delegate) {
self.delegate->OverlayUIDidFinishDismissal(self.request);
}
UpdateBannerAccessibilityForDismissal(self.baseViewController);
}
......
......@@ -138,10 +138,15 @@ class OverlayPresentationContextImpl : public OverlayPresentationContext {
// Called when the UI for |request_| has finished being dismissed.
void OverlayUIWasDismissed();
// Called when the Browser is being destroyed.
void BrowserDestroyed();
// Helper object that detaches the UI delegate for Browser shudown.
class BrowserShutdownHelper : public BrowserObserver {
public:
BrowserShutdownHelper(Browser* browser, OverlayPresenter* presenter);
BrowserShutdownHelper(Browser* browser,
OverlayPresenter* presenter,
OverlayPresentationContextImpl* presentation_context);
~BrowserShutdownHelper() override;
// BrowserObserver:
......@@ -150,6 +155,8 @@ class OverlayPresentationContextImpl : public OverlayPresentationContext {
private:
// The presenter whose delegate needs to be reset.
OverlayPresenter* presenter_ = nullptr;
// OverlayPresentationContextImpl reference.
OverlayPresentationContextImpl* presentation_context_ = nullptr;
};
// Helper object that listens for UI dismissal events.
......
......@@ -68,7 +68,7 @@ OverlayPresentationContextImpl::OverlayPresentationContextImpl(
OverlayModality modality,
OverlayRequestCoordinatorFactory* factory)
: presenter_(OverlayPresenter::FromBrowser(browser, modality)),
shutdown_helper_(browser, presenter_),
shutdown_helper_(browser, presenter_, this),
coordinator_delegate_(this),
fullscreen_disabler_(browser, modality),
coordinator_factory_(factory),
......@@ -374,12 +374,21 @@ void OverlayPresentationContextImpl::OverlayUIWasDismissed() {
SetRequest(nullptr);
}
void OverlayPresentationContextImpl::BrowserDestroyed() {
for (std::pair<OverlayRequest* const, std::unique_ptr<OverlayRequestUIState>>&
state : states_) {
OverlayRequestUIState* ui_state = state.second.get();
ui_state->coordinator().delegate = nil;
}
}
#pragma mark BrowserShutdownHelper
OverlayPresentationContextImpl::BrowserShutdownHelper::BrowserShutdownHelper(
Browser* browser,
OverlayPresenter* presenter)
: presenter_(presenter) {
OverlayPresenter* presenter,
OverlayPresentationContextImpl* presentation_context)
: presenter_(presenter), presentation_context_(presentation_context) {
DCHECK(presenter_);
browser->AddObserver(this);
}
......@@ -390,6 +399,7 @@ OverlayPresentationContextImpl::BrowserShutdownHelper::
void OverlayPresentationContextImpl::BrowserShutdownHelper::BrowserDestroyed(
Browser* browser) {
presenter_->SetPresentationContext(nullptr);
presentation_context_->BrowserDestroyed();
browser->RemoveObserver(this);
}
......
......@@ -38,7 +38,7 @@ class OverlayRequest;
// communicate when the overlay UI is finished being presented and dismissed.
// Overlay UI presentation and dismissal may occur after |-start| and |-stop|,
// even if the overlay is stopped without animation.
@property(nonatomic, readonly) OverlayRequestCoordinatorDelegate* delegate;
@property(nonatomic, assign) OverlayRequestCoordinatorDelegate* delegate;
// The request used to configure the overlay UI.
@property(nonatomic, readonly) OverlayRequest* request;
......
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