Changeset 107118 in webkit


Ignore:
Timestamp:
Feb 8, 2012 12:20:41 PM (12 years ago)
Author:
aestes@apple.com
Message:

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.

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.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r107114 r107118  
     12012-02-07  Andy Estes  <aestes@apple.com>
     2
     3        REGRESSION (r102983): ClicktoFlash drawing of old style youtube embeds missing until resize
     4        https://bugs.webkit.org/show_bug.cgi?id=77167
     5
     6        Reviewed by Eric Seidel.
     7
     8        * plugins/layout-in-beforeload-listener-affects-plugin-loading-expected.txt: Added.
     9        * plugins/layout-in-beforeload-listener-affects-plugin-loading.html: Added.
     10
    1112012-02-08  Cary Clark  <caryclark@google.com>
    212
  • trunk/Source/WebCore/ChangeLog

    r107112 r107118  
     12012-02-07  Andy Estes  <aestes@apple.com>
     2
     3        REGRESSION (r102983): ClicktoFlash drawing of old style youtube embeds missing until resize
     4        https://bugs.webkit.org/show_bug.cgi?id=77167
     5
     6        Reviewed by Eric Seidel.
     7
     8        Test: plugins/layout-in-beforeload-listener-affects-plugin-loading.html
     9
     10        r102983 made FrameView::updateWidgets() check if the DOM node actually
     11        needs a widget update before calling updateWidget(). Due to historical
     12        reasons, however, updateWidget() can be legitimately called twice: once
     13        at attach time for non-Netscape plug-ins and once at layout time for
     14        Netscape plug-ins.
     15
     16        If the widget represents a Netscape plug-in, but updateWidget() is
     17        called for the CreateOnlyNonNetscapePlugins case after the DOM node was
     18        marked as needing an update, updateWidget() will clear the update flag
     19        and prevent a second call to updateWidget() at layout time for the
     20        CreateAnyWidgetType case.
     21
     22        As much as I loathe adding to the code duplication between
     23        HTMLEmbedElement::updateWidget() and HTMLObjectElement::updateWidget(),
     24        the simplest solution seems to be marking the DOM node as needing
     25        update in the case where we are calling updateWidget() for the
     26        CreateOnlyNonNetscapePlugins case and we know we will be loading a
     27        Netscape plug-in.
     28
     29        * html/HTMLEmbedElement.cpp:
     30        (WebCore::HTMLEmbedElement::updateWidget): Call
     31        setNeedsWidgetUpdate(true) if pluginCreationOption is
     32        CreateOnlyNonNetscapePlugins but we will load a Netscape plug-in.
     33        * html/HTMLObjectElement.cpp:
     34        (WebCore::HTMLObjectElement::updateWidget): Ditto.
     35        * html/HTMLPlugInElement.cpp:
     36        (WebCore::HTMLPlugInElement::guardedDispatchBeforeLoadEvent): Remove an
     37        invalid assertion that prevents the layout test from running in a Debug
     38        configuration.
     39
    1402012-02-07  Ojan Vafai  <ojan@chromium.org>
    241
  • trunk/Source/WebCore/html/HTMLEmbedElement.cpp

    r106833 r107118  
    134134    if (!allowedToLoadFrameURL(m_url))
    135135        return;
     136
    136137    // FIXME: It's sadness that we have this special case here.
    137138    //        See http://trac.webkit.org/changeset/25128 and
    138139    //        plugins/netscape-plugin-setwindow-size.html
    139     if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(m_url, m_serviceType))
    140         return;
     140    if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(m_url, m_serviceType)) {
     141        // Ensure updateWidget() is called again during layout to create the Netscape plug-in.
     142        setNeedsWidgetUpdate(true);
     143        return;
     144    }
    141145
    142146    // FIXME: These should be joined into a PluginParameters class.
  • trunk/Source/WebCore/html/HTMLObjectElement.cpp

    r106833 r107118  
    286286    renderEmbeddedObject()->setHasFallbackContent(fallbackContent);
    287287
    288     if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType))
     288    // FIXME: It's sadness that we have this special case here.
     289    //        See http://trac.webkit.org/changeset/25128 and
     290    //        plugins/netscape-plugin-setwindow-size.html
     291    if (pluginCreationOption == CreateOnlyNonNetscapePlugins && wouldLoadAsNetscapePlugin(url, serviceType)) {
     292        // Ensure updateWidget() is called again during layout to create the Netscape plug-in.
     293        setNeedsWidgetUpdate(true);
    289294        return;
     295    }
    290296
    291297    RefPtr<HTMLObjectElement> protect(this); // beforeload and plugin loading can make arbitrary DOM mutations.
  • trunk/Source/WebCore/html/HTMLPlugInElement.cpp

    r106769 r107118  
    113113bool HTMLPlugInElement::guardedDispatchBeforeLoadEvent(const String& sourceURL)
    114114{
    115     ASSERT(!m_inBeforeLoadEventHandler);
     115    // FIXME: Our current plug-in loading design can't guarantee the following
     116    // assertion is true, since plug-in loading can be initiated during layout,
     117    // and synchronous layout can be initiated in a beforeload event handler!
     118    // See <http://webkit.org/b/71264>.
     119    // ASSERT(!m_inBeforeLoadEventHandler);
    116120    m_inBeforeLoadEventHandler = true;
    117121    // static_cast is used to avoid a compile error since dispatchBeforeLoadEvent
Note: See TracChangeset for help on using the changeset viewer.