[webkit-reviews] review denied: [Bug 35550] Allow a plugin to participate in the browser's print workflow. : [Attachment 49757] WebCore changes for plugin print.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Mar 5 12:48:58 PST 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied  review:
Bug 35550: Allow a plugin to participate in the browser's print workflow.
https://bugs.webkit.org/show_bug.cgi?id=35550

Attachment 49757: WebCore changes for plugin print.
https://bugs.webkit.org/attachment.cgi?id=49757&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
> Index: loader/PluginDocument.cpp

> +    static Widget* PluginWidgetFromDocument(Document* doc);

nit: use camelCase naming


> +Widget* PluginTokenizer::PluginWidgetFromDocument(Document* doc)
> +{
> +    ASSERT(doc);
> +    RefPtr<Element> body = doc->body();
> +    if (body) {
> +	   RefPtr<Node> embedNode = body->firstChild();
> +	   RefPtr<HTMLEmbedElement> embedElement =
static_cast<HTMLEmbedElement*>(embedNode.get());
> +	   if (embedElement) {
> +	       RenderWidget* renderer =
toRenderWidget(embedElement->renderer());
> +	       if (renderer)
> +		   return renderer->widget();
> +	   }
> +    }
> +    return 0;
> +}
> +

I would write the above a bit differently.  Like this:

Widget* PluginTokenizer::pluginWidgetFromDocument(Document* doc)
{
    ASSERT(doc);
    RefPtr<Element> body = doc->body();
    if (body) {
	RefPtr<Node> node = body->firstChild();
	if (node && node->renderer())
	    return toRenderEmbeddedObject(node->renderer())->widget();
    }
    return 0;
}

We could also add a check to verify that node->renderer()->isEmbeddedObject()
returns true, but it's also fine to leave that as a debug-only check.  See
the implementation of toRenderEmbeddedObject.


> Index: platform/Widget.h
...
> +    // Virtual methods for printing. The widget can support custom printing
> +    // (which means it controls the layout, number of pages etc). This
typically
> +    // happens if the widget is hosting a plugin. In the below methods,
printableArea
> +    // is in points (a point is 1/72 of an inch).
> +    virtual bool supportsCustomPrint() const { return false; }
> +    virtual int getNumPages(const IntRect& printableArea, int printerDpi)
const { return 0; }
> +    virtual void printPage(int pageNumber, GraphicsContext* gc, const
IntRect& printableArea, int printerDpi) {}

Since you aren't calling these methods from WebCore, it turns out that
there is another option.  From the WebKit layer, if the Document is a
PluginDocument, then we can just get the pluginWidget, and cast that to
WebPluginContainerImpl.  That might be better than adding these methods
to WebCore::Widget.


More information about the webkit-reviews mailing list