WebKit Bugzilla
Attachment 340644 Details for
Bug 185745
: CRASH in -[WKFullScreenViewController _manager]
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-185745-20180517143305.patch (text/plain), 12.01 KB, created by
Jer Noble
on 2018-05-17 14:33:06 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Jer Noble
Created:
2018-05-17 14:33:06 PDT
Size:
12.01 KB
patch
obsolete
>Subversion Revision: 231813 >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index fe4272a976b5b1aec486e99e101c7cbe4549dcfb..2c8226c8162c1d3e791e5aa9dbc82a2965495b3e 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,31 @@ >+2018-05-17 Jer Noble <jer.noble@apple.com> >+ >+ CRASH in -[WKFullScreenViewController _manager] >+ https://bugs.webkit.org/show_bug.cgi?id=185745 >+ <rdar://problem/39195241> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ Protect against WKFullScreenViewController outliving WKWebView by making its >+ _webView property weak. Additionally, add a sanity-check RetainPtr where _webView >+ is referenced multiple times within a function. >+ >+ * UIProcess/ios/fullscreen/WKFullScreenViewController.mm: >+ * UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm: >+ (-[WKFullScreenWindowController initWithWebView:]): >+ (-[WKFullScreenWindowController enterFullScreen]): >+ (-[WKFullScreenWindowController beganEnterFullScreenWithInitialFrame:finalFrame:]): >+ (-[WKFullScreenWindowController beganExitFullScreenWithInitialFrame:finalFrame:]): >+ (-[WKFullScreenWindowController _completedExitFullScreen]): >+ (-[WKFullScreenWindowController close]): >+ (-[WKFullScreenWindowController webViewDidRemoveFromSuperviewWhileInFullscreen]): >+ (-[WKFullScreenWindowController _exitFullscreenImmediately]): >+ (-[WKFullScreenWindowController _isSecure]): >+ (-[WKFullScreenWindowController _serverTrust]): >+ (-[WKFullScreenWindowController _updateLocationInfo]): >+ (-[WKFullScreenWindowController _manager]): >+ (-[WKFullScreenWindowController _startToDismissFullscreenChanged:]): >+ > 2018-05-16 Jer Noble <jer.noble@apple.com> > > Turn Modern EME API on by default and remove it as an experimental feature >diff --git a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm >index 58d09e5f44a77cc5a0e5c735e247c452f8505b76..0c7d48db3157374091cdd59ded4f69cc8912a935 100644 >--- a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm >+++ b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenViewController.mm >@@ -100,7 +100,7 @@ @end > #pragma mark - WKFullScreenViewController > > @interface WKFullScreenViewController () <UIGestureRecognizerDelegate, UIToolbarDelegate> >-@property (assign, nonatomic) WKWebView *_webView; // Cannot be retained, see <rdar://problem/14884666>. >+@property (weak, nonatomic) WKWebView *_webView; // Cannot be retained, see <rdar://problem/14884666>. > @property (readonly, nonatomic) WebFullScreenManagerProxy* _manager; > @property (readonly, nonatomic) CGFloat _effectiveFullscreenInsetTop; > @end >diff --git a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm >index 48884cf7790c063a1b3a5b885f957a9fdf933291..01eb39e75318dae4610ed9b581c50415f18320b9 100644 >--- a/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm >+++ b/Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm >@@ -338,10 +338,10 @@ @end > #pragma mark - > > @interface WKFullScreenWindowController () <UIGestureRecognizerDelegate> >+@property (weak, nonatomic) WKWebView *_webView; // Cannot be retained, see <rdar://problem/14884666>. > @end > > @implementation WKFullScreenWindowController { >- WKWebView *_webView; // Cannot be retained, see <rdar://problem/14884666>. > RetainPtr<UIView> _webViewPlaceholder; > > FullScreenState _fullScreenState; >@@ -374,7 +374,7 @@ - (id)initWithWebView:(WKWebView *)webView > if (!(self = [super init])) > return nil; > >- _webView = webView; >+ self._webView = webView; > > return self; > } >@@ -428,7 +428,9 @@ - (void)enterFullScreen > > _window.get().rootViewController = _rootViewController.get(); > >- _fullscreenViewController = adoptNS([[WKFullScreenViewController alloc] initWithWebView:_webView]); >+ RetainPtr<WKWebView> webView = self._webView; >+ >+ _fullscreenViewController = adoptNS([[WKFullScreenViewController alloc] initWithWebView:webView.get()]); > [_fullscreenViewController setModalPresentationStyle:UIModalPresentationCustom]; > [_fullscreenViewController setTransitioningDelegate:self]; > [_fullscreenViewController setModalPresentationCapturesStatusBarAppearance:YES]; >@@ -451,32 +453,33 @@ - (void)enterFullScreen > > [self _manager]->saveScrollPosition(); > >- [_webView _page]->setSuppressVisibilityUpdates(true); >+ [webView _page]->setSuppressVisibilityUpdates(true); > >- _viewState.store(_webView); >+ _viewState.store(webView.get()); > > _webViewPlaceholder = adoptNS([[UIView alloc] init]); > [[_webViewPlaceholder layer] setName:@"Fullscreen Placeholder View"]; > > WKSnapshotConfiguration* config = nil; >- [_webView takeSnapshotWithConfiguration:config completionHandler:^(UIImage * snapshotImage, NSError * error) { >- if (![_webView _page]) >+ [webView takeSnapshotWithConfiguration:config completionHandler:^(UIImage * snapshotImage, NSError * error) { >+ RetainPtr<WKWebView> webView = self._webView; >+ if (![webView _page]) > return; > > [CATransaction begin]; > [CATransaction setDisableActions:YES]; > > [[_webViewPlaceholder layer] setContents:(id)[snapshotImage CGImage]]; >- replaceViewWithView(_webView, _webViewPlaceholder.get()); >+ replaceViewWithView(webView.get(), _webViewPlaceholder.get()); > >- WKWebViewState().applyTo(_webView); >+ WKWebViewState().applyTo(webView.get()); > >- [_webView setAutoresizingMask:(UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight)]; >- [_webView setFrame:[_window bounds]]; >- [_webView _overrideLayoutParametersWithMinimumLayoutSize:[_window bounds].size maximumUnobscuredSizeOverride:[_window bounds].size]; >- [_window insertSubview:_webView atIndex:0]; >- [_webView setNeedsLayout]; >- [_webView layoutIfNeeded]; >+ [webView setAutoresizingMask:(UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight)]; >+ [webView setFrame:[_window bounds]]; >+ [webView _overrideLayoutParametersWithMinimumLayoutSize:[_window bounds].size maximumUnobscuredSizeOverride:[_window bounds].size]; >+ [_window insertSubview:webView.get() atIndex:0]; >+ [webView setNeedsLayout]; >+ [webView layoutIfNeeded]; > > [self _manager]->setAnimatingFullScreen(true); > >@@ -490,7 +493,7 @@ - (void)enterFullScreen > ASSERT_NOT_REACHED(); > [self _exitFullscreenImmediately]; > }); >- [_webView _page]->forceRepaint(_repaintCallback.copyRef()); >+ [webView _page]->forceRepaint(_repaintCallback.copyRef()); > > [CATransaction commit]; > }]; >@@ -508,7 +511,8 @@ - (void)beganEnterFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CG > [CATransaction begin]; > [CATransaction setDisableActions:YES]; > >- [_webView removeFromSuperview]; >+ RetainPtr<WKWebView> webView = self._webView; >+ [webView removeFromSuperview]; > > [_window setWindowLevel:UIWindowLevelNormal]; > [_window makeKeyAndVisible]; >@@ -520,7 +524,7 @@ - (void)beganEnterFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CG > [_rootViewController presentViewController:_fullscreenViewController.get() animated:YES completion:^{ > _fullScreenState = InFullScreen; > >- auto* page = [_webView _page]; >+ auto* page = [self._webView _page]; > auto* manager = self._manager; > if (page && manager) { > manager->didEnterFullScreen(); >@@ -570,7 +574,7 @@ - (void)beganExitFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CGR > _initialFrame = initialFrame; > _finalFrame = finalFrame; > >- [_webView _page]->setSuppressVisibilityUpdates(true); >+ [self._webView _page]->setSuppressVisibilityUpdates(true); > > [_fullscreenViewController setPrefersStatusBarHidden:NO]; > >@@ -581,7 +585,7 @@ - (void)beganExitFullScreenWithInitialFrame:(CGRect)initialFrame finalFrame:(CGR > } > > [_fullscreenViewController dismissViewControllerAnimated:YES completion:^{ >- if (![_webView _page]) >+ if (![self._webView _page]) > return; > > [self _completedExitFullScreen]; >@@ -597,16 +601,17 @@ - (void)_completedExitFullScreen > [CATransaction begin]; > [CATransaction setDisableActions:YES]; > >- [[_webViewPlaceholder superview] insertSubview:_webView belowSubview:_webViewPlaceholder.get()]; >- [_webView setFrame:[_webViewPlaceholder frame]]; >- [_webView setAutoresizingMask:[_webViewPlaceholder autoresizingMask]]; >+ RetainPtr<WKWebView> webView = self._webView; >+ [[_webViewPlaceholder superview] insertSubview:webView.get() belowSubview:_webViewPlaceholder.get()]; >+ [webView setFrame:[_webViewPlaceholder frame]]; >+ [webView setAutoresizingMask:[_webViewPlaceholder autoresizingMask]]; > >- [[_webView window] makeKeyAndVisible]; >+ [[webView window] makeKeyAndVisible]; > >- _viewState.applyTo(_webView); >+ _viewState.applyTo(webView.get()); > >- [_webView setNeedsLayout]; >- [_webView layoutIfNeeded]; >+ [webView setNeedsLayout]; >+ [webView layoutIfNeeded]; > > [CATransaction commit]; > >@@ -627,13 +632,11 @@ - (void)_completedExitFullScreen > _repaintCallback = nullptr; > [_webViewPlaceholder removeFromSuperview]; > >- if (![_webView _page]) >- return; >- >- [_webView _page]->setSuppressVisibilityUpdates(false); >+ if (auto* page = [self._webView _page]) >+ page->setSuppressVisibilityUpdates(false); > }); > >- if (auto* page = [_webView _page]) >+ if (auto* page = [self._webView _page]) > page->forceRepaint(_repaintCallback.copyRef()); > else > _repaintCallback->performCallback(); >@@ -644,12 +647,12 @@ - (void)_completedExitFullScreen > - (void)close > { > [self _exitFullscreenImmediately]; >- _webView = nil; >+ self._webView = nil; > } > > - (void)webViewDidRemoveFromSuperviewWhileInFullscreen > { >- if (_fullScreenState == InFullScreen && _webView.window != _window.get()) >+ if (_fullScreenState == InFullScreen && self._webView.window != _window.get()) > [self _exitFullscreenImmediately]; > } > >@@ -726,8 +729,9 @@ - (void)_exitFullscreenImmediately > [self exitFullScreen]; > _fullScreenState = ExitingFullScreen; > [self _completedExitFullScreen]; >- replaceViewWithView(_webViewPlaceholder.get(), _webView); >- if (auto* page = [_webView _page]) >+ RetainPtr<WKWebView> webView = self._webView; >+ replaceViewWithView(_webViewPlaceholder.get(), webView.get()); >+ if (auto* page = [webView _page]) > page->setSuppressVisibilityUpdates(false); > if (manager) { > manager->didExitFullScreen(); >@@ -744,12 +748,12 @@ - (void)_invalidateEVOrganizationName > > - (BOOL)_isSecure > { >- return _webView.hasOnlySecureContent; >+ return self._webView.hasOnlySecureContent; > } > > - (SecTrustRef)_serverTrust > { >- return _webView.serverTrust; >+ return self._webView.serverTrust; > } > > - (NSString *)_EVOrganizationName >@@ -795,7 +799,7 @@ - (NSString *)_EVOrganizationName > > - (void)_updateLocationInfo > { >- NSURL* url = _webView._committedURL; >+ NSURL* url = self._webView._committedURL; > > NSString *EVOrganizationName = [self _EVOrganizationName]; > BOOL showsEVOrganizationName = [EVOrganizationName length] > 0; >@@ -824,16 +828,16 @@ - (void)_updateLocationInfo > > - (WebFullScreenManagerProxy*)_manager > { >- if (![_webView _page]) >- return nullptr; >- return [_webView _page]->fullScreenManager(); >+ if (auto* page = [self._webView _page]) >+ return page->fullScreenManager(); >+ return nullptr; > } > > - (void)_startToDismissFullscreenChanged:(id)sender > { > _inInteractiveDismiss = true; > [_fullscreenViewController dismissViewControllerAnimated:YES completion:^{ >- if (![_webView _page]) >+ if (![self._webView _page]) > return; > > [self _completedExitFullScreen];
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 185745
: 340644