Attachment 340644 Details for Bug 185745 – Patch

[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:

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];

Actions: View | Formatted Diff | Diff

Attachments on bug 185745: 340644