Bug 47550 - For WebKit plug-ins, beforeload can be called recursively (esp. with AdBlock style extensions)
Summary: For WebKit plug-ins, beforeload can be called recursively (esp. with AdBlock ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: InRadar
Depends on:
Blocks: 74340
  Show dependency treegraph
 
Reported: 2010-10-12 11:54 PDT by Alexey Proskuryakov
Modified: 2011-12-12 14:43 PST (History)
2 users (show)

See Also:


Attachments
test case (change MIME type to a plug-in you have installed) (622 bytes, text/html)
2010-10-12 11:54 PDT, Alexey Proskuryakov
no flags Details
proposed patch (4.44 KB, patch)
2010-10-12 12:30 PDT, Alexey Proskuryakov
simon.fraser: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2010-10-12 11:54:47 PDT
Created attachment 70550 [details]
test case (change MIME type to a plug-in you have installed)

This happens with AdBlock extension and any WebKit-style plug-in:
1. Add a plug-in element to a document.
2. Access any property, e.g. myPlugin.myProperty. This makes the plug-in load, since myProperty can be defined in the plug-in. As the plug-in is loaded, a beforeload event is dispatched.
3. In beforeload handler, access e.g. myPlugin.nodeName. Since the plug-in hasn't loaded yet, we go back into HTMLObjectElement::updateWidget(), and dispatch beforeload again.

Two of the ways updateWidget is triggered are style resolution and layout. The interaction of these result in one recursive call, and also Widget object is created twice. So, the plug-in is stopped, and malfunctions in the future.

<rdar://problem/8353386>
Comment 1 Alexey Proskuryakov 2010-10-12 12:30:33 PDT
Created attachment 70554 [details]
proposed patch
Comment 2 Simon Fraser (smfr) 2010-10-12 13:12:48 PDT
Comment on attachment 70554 [details]
proposed patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70554&action=review

> WebCore/html/HTMLPlugInElement.cpp:108
> +    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 false;
> +    }

This should return 0, not return false.
Comment 3 Alexey Proskuryakov 2010-10-12 13:29:47 PDT
Committed <http://trac.webkit.org/changeset/69596>.