Bug 237835 – [iOS] Indefinite hang when printing using a UIPrintPageRenderer

• Status: RESOLVED
• Resolution: FIXED
• Component: Printing

[Aditya Keerthi]

...

[Aditya Keerthi]

rdar://90002387

[Aditya Keerthi]

Created attachment 454602 [details]
Patch

[Devin Rousso]

Comment on attachment 454602 [details]
Patch

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

r=me, thanks for fixing!

> Source/WebKit/UIProcess/ios/WKContentView.mm:692
> +    if (_pdfPrintCallbackID) {

NIT: Do we need to check for `_pdfPrintCallbackID` here?  I feel like the inner `if` is enough.

> Source/WebKit/UIProcess/ios/WKContentView.mm:932
> +        _pdfPrintCompletionSemaphore = makeUnique<BinarySemaphore>();

NIT: should we `ASSERT(!_pdfPrintCompletionSemaphore);`?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewPrintFormatter.mm:99
> +TEST(WKWebView, PrintToPDF)

Is there a way for us to test the non-main-thread path as well?

[Aditya Keerthi]

Created attachment 454618 [details]
Patch for landing

[Aditya Keerthi]

(In reply to Devin Rousso from comment #3)
> Comment on attachment 454602 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=454602&action=review
> 
> r=me, thanks for fixing!

Thanks for the review!
 
> > Source/WebKit/UIProcess/ios/WKContentView.mm:692
> > +    if (_pdfPrintCallbackID) {
> 
> NIT: Do we need to check for `_pdfPrintCallbackID` here?  I feel like the
> inner `if` is enough.

Removed, the inner `if` is enough due to the null check.
 
> > Source/WebKit/UIProcess/ios/WKContentView.mm:932
> > +        _pdfPrintCompletionSemaphore = makeUnique<BinarySemaphore>();
> 
> NIT: should we `ASSERT(!_pdfPrintCompletionSemaphore);`?

Added an assertion.
 
> > Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewPrintFormatter.mm:99
> > +TEST(WKWebView, PrintToPDF)
> 
> Is there a way for us to test the non-main-thread path as well?

Added a test which exercises the non-main-thread path using UIPrintInteractionController.

[EWS]

Committed r291304 (248443@main): <https://commits.webkit.org/248443@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 454618 [details].