- Revision
- 174040
- Author
- [email protected]
- Date
- 2014-09-27 22:57:48 -0700 (Sat, 27 Sep 2014)
Log Message
HTMLPlugInElement::isUserObservable() is causing layout
https://bugs.webkit.org/show_bug.cgi?id=137156
Reviewed by Ryosuke Niwa.
While profiling the page load of nytimes.com, I noticed that we were
spending ~4-5% of cpu time in HTMLPlugInElement::isUserObservable().
The reason is that the function calls pluginWidget(), which causes a
layout update in HTMLObjectElement::renderWidgetForJSBindings(), to
make sure the plugin is loaded and its renderer is created.
HTMLPlugInElement::isUserObservable() shouldn't need to do a layout.
This patch does the following to address the problem:
- Rename renderWidgetForJSBindings() to renderWidgetLoadingPlugin()
because this function is not always called from the JS Bindings
nowadays. The new name makes it clearer that this will load the
plugin if needed (to make sure the renderer is created, and by
doing a layout).
- Add a PluginLoadingPolicy argument to
HTMLPlugInElement::pluginWidget() to let the caller control if the
plugin should be loaded or not.
- Update the call to pluginWidget() in isUserObservable() so that
we do not attempt to load the plugin (thus not causing a layout).
No new tests, no behavior change.
* WebCore.exp.in:
* WebCore.order:
* html/HTMLAppletElement.cpp:
(WebCore::HTMLAppletElement::renderWidgetLoadingPlugin):
(WebCore::HTMLAppletElement::renderWidgetForJSBindings): Deleted.
* html/HTMLAppletElement.h:
* html/HTMLEmbedElement.cpp:
(WebCore::HTMLEmbedElement::renderWidgetLoadingPlugin):
(WebCore::HTMLEmbedElement::renderWidgetForJSBindings): Deleted.
* html/HTMLEmbedElement.h:
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::renderWidgetLoadingPlugin):
(WebCore::HTMLObjectElement::renderWidgetForJSBindings): Deleted.
* html/HTMLObjectElement.h:
* html/HTMLPlugInElement.cpp:
(WebCore::HTMLPlugInElement::pluginWidget):
(WebCore::HTMLPlugInElement::isUserObservable):
* html/HTMLPlugInElement.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (174039 => 174040)
--- trunk/Source/WebCore/ChangeLog 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/ChangeLog 2014-09-28 05:57:48 UTC (rev 174040)
@@ -1,3 +1,50 @@
+2014-09-27 Chris Dumez <[email protected]>
+
+ HTMLPlugInElement::isUserObservable() is causing layout
+ https://bugs.webkit.org/show_bug.cgi?id=137156
+
+ Reviewed by Ryosuke Niwa.
+
+ While profiling the page load of nytimes.com, I noticed that we were
+ spending ~4-5% of cpu time in HTMLPlugInElement::isUserObservable().
+ The reason is that the function calls pluginWidget(), which causes a
+ layout update in HTMLObjectElement::renderWidgetForJSBindings(), to
+ make sure the plugin is loaded and its renderer is created.
+
+ HTMLPlugInElement::isUserObservable() shouldn't need to do a layout.
+ This patch does the following to address the problem:
+ - Rename renderWidgetForJSBindings() to renderWidgetLoadingPlugin()
+ because this function is not always called from the JS Bindings
+ nowadays. The new name makes it clearer that this will load the
+ plugin if needed (to make sure the renderer is created, and by
+ doing a layout).
+ - Add a PluginLoadingPolicy argument to
+ HTMLPlugInElement::pluginWidget() to let the caller control if the
+ plugin should be loaded or not.
+ - Update the call to pluginWidget() in isUserObservable() so that
+ we do not attempt to load the plugin (thus not causing a layout).
+
+ No new tests, no behavior change.
+
+ * WebCore.exp.in:
+ * WebCore.order:
+ * html/HTMLAppletElement.cpp:
+ (WebCore::HTMLAppletElement::renderWidgetLoadingPlugin):
+ (WebCore::HTMLAppletElement::renderWidgetForJSBindings): Deleted.
+ * html/HTMLAppletElement.h:
+ * html/HTMLEmbedElement.cpp:
+ (WebCore::HTMLEmbedElement::renderWidgetLoadingPlugin):
+ (WebCore::HTMLEmbedElement::renderWidgetForJSBindings): Deleted.
+ * html/HTMLEmbedElement.h:
+ * html/HTMLObjectElement.cpp:
+ (WebCore::HTMLObjectElement::renderWidgetLoadingPlugin):
+ (WebCore::HTMLObjectElement::renderWidgetForJSBindings): Deleted.
+ * html/HTMLObjectElement.h:
+ * html/HTMLPlugInElement.cpp:
+ (WebCore::HTMLPlugInElement::pluginWidget):
+ (WebCore::HTMLPlugInElement::isUserObservable):
+ * html/HTMLPlugInElement.h:
+
2014-09-27 Christophe Dumez <[email protected]>
Use the new is<>() / downcast<>() for more Node subclasses
Modified: trunk/Source/WebCore/WebCore.exp.in (174039 => 174040)
--- trunk/Source/WebCore/WebCore.exp.in 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/WebCore.exp.in 2014-09-28 05:57:48 UTC (rev 174040)
@@ -1785,7 +1785,6 @@
__ZNK7WebCore16VisibleSelection23isContentRichlyEditableEv
__ZNK7WebCore16VisibleSelection5isAllENS_27EditingBoundaryCrossingRuleE
__ZNK7WebCore17HTMLOptionElement4textEv
-__ZNK7WebCore17HTMLPlugInElement12pluginWidgetEv
__ZNK7WebCore17HTMLSelectElement13selectedIndexEv
__ZNK7WebCore17HTMLSelectElement5valueEv
__ZNK7WebCore17HTMLSelectElement9listItemsEv
Modified: trunk/Source/WebCore/WebCore.order (174039 => 174040)
--- trunk/Source/WebCore/WebCore.order 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/WebCore.order 2014-09-28 05:57:48 UTC (rev 174040)
@@ -9187,7 +9187,6 @@
__ZN7WebCore37pluginElementCustomGetOwnPropertySlotINS_19JSHTMLObjectElementENS_13JSHTMLElementEEEbPN3JSC9ExecStateENS3_12PropertyNameERNS3_12PropertySlotEPT_
__ZN7WebCore37runtimeObjectCustomGetOwnPropertySlotEPN3JSC9ExecStateENS0_12PropertyNameERNS0_12PropertySlotEPNS_13JSHTMLElementE
__ZN7WebCore18pluginScriptObjectEPN3JSC9ExecStateEPNS_13JSHTMLElementE
-__ZNK7WebCore17HTMLPlugInElement12pluginWidgetEv
__ZNK7WebCore17HTMLObjectElement25renderWidgetForJSBindingsEv
__ZN7WebCore17HTMLPlugInElement11getInstanceEv
__ZN7WebCore28JSHTMLObjectElementPrototype18getOwnPropertySlotEPN3JSC6JSCellEPNS1_9ExecStateENS1_12PropertyNameERNS1_12PropertySlotE
Modified: trunk/Source/WebCore/html/HTMLAppletElement.cpp (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLAppletElement.cpp 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLAppletElement.cpp 2014-09-28 05:57:48 UTC (rev 174040)
@@ -83,10 +83,10 @@
return RenderEmbeddedObject::createForApplet(*this, WTF::move(style));
}
-RenderWidget* HTMLAppletElement::renderWidgetForJSBindings() const
+RenderWidget* HTMLAppletElement::renderWidgetLoadingPlugin() const
{
if (!canEmbedJava())
- return 0;
+ return nullptr;
// Needs to load the plugin immediatedly because this function is called
// when _javascript_ code accesses the plugin.
Modified: trunk/Source/WebCore/html/HTMLAppletElement.h (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLAppletElement.h 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLAppletElement.h 2014-09-28 05:57:48 UTC (rev 174040)
@@ -39,7 +39,7 @@
virtual bool rendererIsNeeded(const RenderStyle&) override;
virtual RenderPtr<RenderElement> createElementRenderer(PassRef<RenderStyle>) override;
- virtual RenderWidget* renderWidgetForJSBindings() const override;
+ virtual RenderWidget* renderWidgetLoadingPlugin() const override;
virtual void updateWidget(PluginCreationOption) override;
bool canEmbedJava() const;
Modified: trunk/Source/WebCore/html/HTMLEmbedElement.cpp (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLEmbedElement.cpp 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLEmbedElement.cpp 2014-09-28 05:57:48 UTC (rev 174040)
@@ -68,7 +68,7 @@
return 0;
}
-RenderWidget* HTMLEmbedElement::renderWidgetForJSBindings() const
+RenderWidget* HTMLEmbedElement::renderWidgetLoadingPlugin() const
{
FrameView* view = document().view();
if (!view || (!view->isInLayout() && !view->isPainting())) {
Modified: trunk/Source/WebCore/html/HTMLEmbedElement.h (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLEmbedElement.h 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLEmbedElement.h 2014-09-28 05:57:48 UTC (rev 174040)
@@ -43,7 +43,7 @@
virtual bool isURLAttribute(const Attribute&) const override;
virtual const AtomicString& imageSourceURL() const override;
- virtual RenderWidget* renderWidgetForJSBindings() const override;
+ virtual RenderWidget* renderWidgetLoadingPlugin() const override;
virtual void updateWidget(PluginCreationOption) override;
Modified: trunk/Source/WebCore/html/HTMLObjectElement.cpp (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLObjectElement.cpp 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLObjectElement.cpp 2014-09-28 05:57:48 UTC (rev 174040)
@@ -81,7 +81,7 @@
return adoptRef(new HTMLObjectElement(tagName, document, form, createdByParser));
}
-RenderWidget* HTMLObjectElement::renderWidgetForJSBindings() const
+RenderWidget* HTMLObjectElement::renderWidgetLoadingPlugin() const
{
// Needs to load the plugin immediatedly because this function is called
// when _javascript_ code accesses the plugin.
Modified: trunk/Source/WebCore/html/HTMLObjectElement.h (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLObjectElement.h 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLObjectElement.h 2014-09-28 05:57:48 UTC (rev 174040)
@@ -70,7 +70,7 @@
virtual bool isURLAttribute(const Attribute&) const override;
virtual const AtomicString& imageSourceURL() const override;
- virtual RenderWidget* renderWidgetForJSBindings() const override;
+ virtual RenderWidget* renderWidgetLoadingPlugin() const override;
virtual void addSubresourceAttributeURLs(ListHashSet<URL>&) const override;
Modified: trunk/Source/WebCore/html/HTMLPlugInElement.cpp (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLPlugInElement.cpp 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.cpp 2014-09-28 05:57:48 UTC (rev 174040)
@@ -158,17 +158,17 @@
return beforeLoadAllowedLoad;
}
-Widget* HTMLPlugInElement::pluginWidget() const
+Widget* HTMLPlugInElement::pluginWidget(PluginLoadingPolicy loadPolicy) const
{
if (m_inBeforeLoadEventHandler) {
// The plug-in hasn't loaded yet, and it makes no sense to try to load if beforeload handler happened to touch the plug-in element.
// That would recursively call beforeload for the same element.
- return 0;
+ return nullptr;
}
- RenderWidget* renderWidget = renderWidgetForJSBindings();
+ RenderWidget* renderWidget = loadPolicy == PluginLoadingPolicy::Load ? renderWidgetLoadingPlugin() : this->renderWidget();
if (!renderWidget)
- return 0;
+ return nullptr;
return renderWidget->widget();
}
@@ -256,7 +256,7 @@
bool HTMLPlugInElement::isUserObservable() const
{
// No widget - can't be anything to see or hear here.
- Widget* widget = pluginWidget();
+ Widget* widget = pluginWidget(PluginLoadingPolicy::DoNotLoad);
if (!widget || !widget->isPluginViewBase())
return false;
Modified: trunk/Source/WebCore/html/HTMLPlugInElement.h (174039 => 174040)
--- trunk/Source/WebCore/html/HTMLPlugInElement.h 2014-09-28 05:21:30 UTC (rev 174039)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.h 2014-09-28 05:57:48 UTC (rev 174040)
@@ -51,7 +51,8 @@
PassRefPtr<JSC::Bindings::Instance> getInstance();
- WEBCORE_EXPORT Widget* pluginWidget() const;
+ enum class PluginLoadingPolicy { DoNotLoad, Load };
+ WEBCORE_EXPORT Widget* pluginWidget(PluginLoadingPolicy = PluginLoadingPolicy::Load) const;
enum DisplayState {
WaitingForSnapshot,
@@ -116,7 +117,8 @@
bool dispatchBeforeLoadEvent(const String& sourceURL); // Not implemented, generates a compile error if subclasses call this by mistake.
- virtual RenderWidget* renderWidgetForJSBindings() const = 0;
+ // This will load the plugin if necessary.
+ virtual RenderWidget* renderWidgetLoadingPlugin() const = 0;
virtual bool supportsFocus() const override;
