[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