[Webkit-unassigned] [Bug 72801] Cannot print USPS shipping labels

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=72801





--- Comment #8 from mitz at webkit.org  2011-12-31 12:18:08 PST ---
(From update of attachment 120761)
View in context: https://bugs.webkit.org/attachment.cgi?id=120761&action=review

>> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp:838
>> +    if (!page)
> 
> We have a check for pageless frame here, but not in BuiltInPDFView::initialize() or BuiltInPDFView::isActive(). Can page actually be null?

I’m quite certain it *cannot* be null in initialize(). I’m going to be paranoid and add a null check in isActive() rather than try to reason about lots of code outside this class. In this function, in particular, I don’t want to make any assumptions, in case Doc is ever extended to have something like setTimeout.

>> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.cpp:841
>> +    page->sendSync(Messages::WebPageProxy::PrintFrame(frame->frameID()), Messages::WebPageProxy::PrintFrame::Reply());
> 
> I'd have routed this through WebCore, for instance to make use of PageGroupLoadDeferrer once Chrome::print starts using it.

Good idea. I’ll do that.

>> Source/WebKit2/WebProcess/Plugins/PDF/BuiltInPDFView.h:35
>> +typedef const struct OpaqueJSValue* JSValueRef;
> 
> :(

You’ll thank me when JavaScriptCore public headers change and you don’t have to recompile all files that include this header!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.