Bug 237835

Summary:

[iOS] Indefinite hang when printing using a UIPrintPageRenderer

Product:

WebKit

Reporter:

Aditya Keerthi <akeerthi>

Component:

Printing

Assignee:

Aditya Keerthi <akeerthi>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

benjamin, cdumez, cmarcelo, ews-watchlist, hi, katherine_cheney, megan_gardner, thorton, webkit-bug-importer, wenson_hsieh

Priority:

Keywords:

InRadar

Version:

Other

Hardware:

iPhone / iPad

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Aditya Keerthi

Reported 2022-03-14 10:00:49 PDT

...

Attachments
Patch (19.17 KB, patch) 2022-03-14 10:04 PDT, Aditya Keerthi	no flags	Details Formatted Diff Diff
Patch for landing (21.05 KB, patch) 2022-03-14 13:30 PDT, Aditya Keerthi	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Aditya Keerthi

Comment 1 2022-03-14 10:01:04 PDT

rdar://90002387

Aditya Keerthi

Comment 2 2022-03-14 10:04:03 PDT

Created attachment 454602 [details] Patch

Devin Rousso

Comment 3 2022-03-14 10:54:37 PDT

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

Comment 4 2022-03-14 13:30:24 PDT

Created attachment 454618 [details] Patch for landing

Aditya Keerthi

Comment 5 2022-03-14 13:32:42 PDT

(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

Comment 6 2022-03-15 10:40:40 PDT

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

Note You need to log in before you can comment on or make changes to this bug.