Bug 101382 – [WK2] Print preview should vend images to the UIProcess instead of PDFs

• Component: PDF
• Status: RESOLVED FIXED

[Tim Horton]

To achieve this, I'm going to have the plugin vend print-preview bitmaps to the UIProcess to draw when previewing.

[Tim Horton]

<rdar://problem/9866766>

[Tim Horton]

Retitling based on clarification.

[Tim Horton]

Created attachment 172874 [details]
patch

[WebKit Review Bot]

Attachment 172874 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/UIProcess/GenericCallback.h:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Alexey Proskuryakov]

Comment on attachment 172874 [details]
patch

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

Looks great!

You might want to additionally run this by Sam or Anders.

> Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:218
> +    // If the user has already changed print setup, then this response is obsolete. And this callback is not in response to the latest request,

I think that there is something wrong with grammar of this moved comment.

> Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:474
> +    HashMap<WebCore::IntRect, RefPtr<WebImage> >::iterator pagePreviewIterator = _pagePreviews.find(rect);

WebImage looks like an API type that shouldn't be used here.

> Source/WebKit2/UIProcess/API/mac/WKPrintingView.mm:517
> +    const IntRect& imageBounds = imageData->bitmap()->bounds();

Does this const reference buy us any performance over a plain IntRect (not that this code is performance critical anyway)?

> Source/WebKit2/UIProcess/GenericCallback.h:237
> +    : CallbackBase(context)
> +    , m_callback(callback)

Indentation.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3198
> +            drawRectToPDFFromPDFDocument(graphicsContext->platformContext(), pdfDocument.get(), printInfo, rect);

Please rename this function.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3203
> +            image = scaledSnapshotWithOptions(rect, 1.0, SnapshotOptionsShareable | SnapshotOptionsExcludeSelectionHighlighting);

Style nit: 1, not 1.0.

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3209
> +    if (!image)
> +        return;

UI process needs its response back!

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3212
> +    image->bitmap()->createHandle(handle);

This creates a read-write handle. I wonder why.

[Tim Horton]

Created attachment 172917 [details]
address ap's comments

Have an r+ from Alexey, would like at least a cursory review from Sam or Anders.

[WebKit Review Bot]

Attachment 172917 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1
Source/WebKit2/UIProcess/API/mac/WKPrintingView.h:33:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Tim Horton]

Thanks, all!

http://trac.webkit.org/changeset/133935