https://bugs.webkit.org/show_bug.cgi?id=77167
Reviewed by Eric Seidel.
Source/WebCore:
Test: plugins/layout-in-beforeload-listener-affects-plugin-loading.html
r102983 made FrameView::updateWidgets() check if the DOM node actually
needs a widget update before calling updateWidget(). Due to historical
reasons, however, updateWidget() can be legitimately called twice: once
at attach time for non-Netscape plug-ins and once at layout time for
Netscape plug-ins.
If the widget represents a Netscape plug-in, but updateWidget() is
called for the CreateOnlyNonNetscapePlugins case after the DOM node was
marked as needing an update, updateWidget() will clear the update flag
and prevent a second call to updateWidget() at layout time for the
CreateAnyWidgetType case.
As much as I loathe adding to the code duplication between
HTMLEmbedElement::updateWidget() and HTMLObjectElement::updateWidget(),
the simplest solution seems to be marking the DOM node as needing
update in the case where we are calling updateWidget() for the
CreateOnlyNonNetscapePlugins case and we know we will be loading a
Netscape plug-in.
* html/HTMLEmbedElement.cpp:
(WebCore::HTMLEmbedElement::updateWidget): Call
setNeedsWidgetUpdate(true) if pluginCreationOption is
CreateOnlyNonNetscapePlugins but we will load a Netscape plug-in.
* html/HTMLObjectElement.cpp:
(WebCore::HTMLObjectElement::updateWidget): Ditto.
* html/HTMLPlugInElement.cpp:
(WebCore::HTMLPlugInElement::guardedDispatchBeforeLoadEvent): Remove an
invalid assertion that prevents the layout test from running in a Debug
configuration.
LayoutTests:
* plugins/layout-in-beforeload-listener-affects-plugin-loading-expected.txt: Added.
* plugins/layout-in-beforeload-listener-affects-plugin-loading.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107118
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-02-07 Andy Estes <aestes@apple.com>
+
+ REGRESSION (r102983): ClicktoFlash drawing of old style youtube embeds missing until resize
+ https://bugs.webkit.org/show_bug.cgi?id=77167
+
+ Reviewed by Eric Seidel.
+
+ * plugins/layout-in-beforeload-listener-affects-plugin-loading-expected.txt: Added.
+ * plugins/layout-in-beforeload-listener-affects-plugin-loading.html: Added.
+
2012-02-08 Cary Clark <caryclark@google.com>
Unreviewed rebaselines for vertical text tests.
--- /dev/null
+CONSOLE MESSAGE: line 27: PASS
+
--- /dev/null
+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ window._beforeloadReceivedForEmbed = false;
+
+ document.addEventListener("beforeload", function(event) {
+ if (document.getElementsByTagName("object")[0] == event.target) {
+ // trigger a layout
+ event.target.offsetWidth;
+ return;
+ }
+
+ _beforeloadReceivedForEmbed = document.getElementsByTagName("embed")[0] == event.target;
+ }, true);
+
+ function test() {
+ // The <embed> should load as a post-layout task that executes in
+ // the current run loop iteration. Queue a task to check if the
+ // <embed>'s beforeload event fired.
+ window.setTimeout(function() {
+ console.log(_beforeloadReceivedForEmbed ? "PASS" : "FAIL");
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }, 0);
+ }
+ </script>
+</head>
+<body onload="test();">
+ <!-- Test that performing layout in a beforeload listener does not affect
+ plug-in loading. See http://webkit.org/b/77167 for details. This test
+ requires TestNetscapePlugIn so it must be run in DumpRenderTree. -->
+ <object>
+ <embed type="application/x-webkit-test-netscape">
+ </object>
+</body>
+</html>
\ No newline at end of file
+2012-02-07 Andy Estes <aestes@apple.com>
+
+ REGRESSION (r102983): ClicktoFlash drawing of old style youtube embeds missing until resize
+ https://bugs.webkit.org/show_bug.cgi?id=77167
+
+ Reviewed by Eric Seidel.
+
+ Test: plugins/layout-in-beforeload-listener-affects-plugin-loading.html
+
+ r102983 made FrameView::updateWidgets() check if the DOM node actually
+ needs a widget update before calling updateWidget(). Due to historical
+ reasons, however, updateWidget() can be legitimately called twice: once
+ at attach time for non-Netscape plug-ins and once at layout time for
+ Netscape plug-ins.
+
+ If the widget represents a Netscape plug-in, but updateWidget() is
+ called for the CreateOnlyNonNetscapePlugins case after the DOM node was
+ marked as needing an update, updateWidget() will clear the update flag
+ and prevent a second call to updateWidget() at layout time for the
+ CreateAnyWidgetType case.
+
+ As much as I loathe adding to the code duplication between
+ HTMLEmbedElement::updateWidget() and HTMLObjectElement::updateWidget(),
+ the simplest solution seems to be marking the DOM node as needing
+ update in the case where we are calling updateWidget() for the
+ CreateOnlyNonNetscapePlugins case and we know we will be loading a
+ Netscape plug-in.
+
+ * html/HTMLEmbedElement.cpp:
+ (WebCore::HTMLEmbedElement::updateWidget): Call
+ setNeedsWidgetUpdate(true) if pluginCreationOption is
+ CreateOnlyNonNetscapePlugins but we will load a Netscape plug-in.
+ * html/HTMLObjectElement.cpp:
+ (WebCore::HTMLObjectElement::updateWidget): Ditto.
+ * html/HTMLPlugInElement.cpp:
+ (WebCore::HTMLPlugInElement::guardedDispatchBeforeLoadEvent): Remove an
+ invalid assertion that prevents the layout test from running in a Debug
+ configuration.
+
2012-02-07 Ojan Vafai <ojan@chromium.org>
Floated flexboxes render as regular RenderBlocks
// <object> which modifies url and serviceType before calling these.
if (!allowedToLoadFrameURL(m_url))
return;
+
// FIXME: It's sadness that we have this special case here.
// See http://trac.webkit.org/changeset/25128 and
// plugins/netscape-plugin-setwindow-size.html
- if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(m_url, m_serviceType))
+ if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(m_url, m_serviceType)) {
+ // Ensure updateWidget() is called again during layout to create the Netscape plug-in.
+ setNeedsWidgetUpdate(true);
return;
+ }
// FIXME: These should be joined into a PluginParameters class.
Vector<String> paramNames;
bool fallbackContent = hasFallbackContent();
renderEmbeddedObject()->setHasFallbackContent(fallbackContent);
- if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType))
+ // FIXME: It's sadness that we have this special case here.
+ // See http://trac.webkit.org/changeset/25128 and
+ // plugins/netscape-plugin-setwindow-size.html
+ if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType)) {
+ // Ensure updateWidget() is called again during layout to create the Netscape plug-in.
+ setNeedsWidgetUpdate(true);
return;
+ }
RefPtr<HTMLObjectElement> protect(this); // beforeload and plugin loading can make arbitrary DOM mutations.
bool beforeLoadAllowedLoad = guardedDispatchBeforeLoadEvent(url);
bool HTMLPlugInElement::guardedDispatchBeforeLoadEvent(const String& sourceURL)
{
- ASSERT(!m_inBeforeLoadEventHandler);
+ // FIXME: Our current plug-in loading design can't guarantee the following
+ // assertion is true, since plug-in loading can be initiated during layout,
+ // and synchronous layout can be initiated in a beforeload event handler!
+ // See <http://webkit.org/b/71264>.
+ // ASSERT(!m_inBeforeLoadEventHandler);
m_inBeforeLoadEventHandler = true;
// static_cast is used to avoid a compile error since dispatchBeforeLoadEvent
// is intentionally undefined on this class.