Bug 171090 – WebContent process becomes unresponsive after returning nil from async version of -webView:createWebViewWithConfiguration:...

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Brady Eidson]

WebContent process becomes unresponsive after returning nil from async version of -webView:createWebViewWithConfiguration:...

<rdar://problem/31739023>

[Brady Eidson]

Created attachment 307664 [details]
Patch

[Sam Weinig]

Comment on attachment 307664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307664&action=review

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:198
> +            if (webView && [webView->_configuration _relatedWebView] != relatedWebView.get())
>                  [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];

If this exception is caught, does it leave the WebProcess in an unresponsive state as well?

[Andy Estes]

Comment on attachment 307664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307664&action=review

> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:-198
> -            if (!webView)
> -                return;

Can you call completionHandler with nullptr here and then not bother with the null checks below?

> Tools/TestWebKitAPI/Tests/WebKit2Cocoa/OpenAndCloseWindow.mm:180
>  // https://bugs.webkit.org/show_bug.cgi?id=171083 - Try to figure out why this fails for some configs but not others, and resolve.
>  //TEST(WebKit2, OpenAndCloseWindowAsyncCallbackException)
>  //{
> -//    openedWebView = nullptr;
> -//    isDone = false;
> +//    resetToConsistentState();
>  //
>  //    RetainPtr<WKWebView> webView = adoptNS([[WKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
>  //

Commented out code!

[Brady Eidson]

Comment on attachment 307664 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=307664&action=review

>> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:198
>>                  [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];
> 
> If this exception is caught, does it leave the WebProcess in an unresponsive state as well?

No.

Though it's a good point here...  should we allow a bogusly created WebView through just because the app caught the exception?

Same possibility has always existed for the synchronous form of this API, so it's not a new issue.

[Brady Eidson]

(In reply to Andy Estes from comment #3)
> Comment on attachment 307664 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307664&action=review
> 
> > Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:-198
> > -            if (!webView)
> > -                return;
> 
> Can you call completionHandler with nullptr here and then not bother with
> the null checks below?

Yup!

[Brady Eidson]

(In reply to Brady Eidson from comment #4)
> Comment on attachment 307664 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307664&action=review
> 
> >> Source/WebKit2/UIProcess/Cocoa/UIDelegate.mm:198
> >>                  [NSException raise:NSInternalInconsistencyException format:@"Returned WKWebView was not created with the given configuration."];
> > 
> > If this exception is caught, does it leave the WebProcess in an unresponsive state as well?
> 
> No.
> 
> Though it's a good point here...  should we allow a bogusly created WebView
> through just because the app caught the exception?
> 
> Same possibility has always existed for the synchronous form of this API, so
> it's not a new issue.


Nevermind, it would totally hang and you'd be left in a completely inconsistent state...

But same thing for the sync version of this API, as well.

[Brady Eidson]

Created attachment 307667 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 307667 [details]
Patch

Clearing flags on attachment: 307667

Committed r215598: <http://trac.webkit.org/changeset/215598>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.