Bug 114994 - Plugin Snapshotting: Don't consume insane amounts of time detecting the primary plugin
Summary: Plugin Snapshotting: Don't consume insane amounts of time detecting the prima...
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
Keywords: InRadar
Depends on:
Reported: 2013-04-22 15:13 PDT by Tim Horton
Modified: 2013-04-22 15:35 PDT (History)
3 users (show)

See Also:

patch (4.76 KB, patch)
2013-04-22 15:18 PDT, Tim Horton
andersca: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2013-04-22 15:13:28 PDT
Some pages either have tons of subframes or dynamically add lots of plugins.

We're currently wasting a lot of time on every subframe load and every addPluginView doing primary plugin detection. Some things we can do to improve this:

* only start detecting after the main frame's didFinishLoad comes. This was intended to be in the original implementation, but we only wait for the first subframe's didFinishLoad instead :|

* only explicitly detect on the main frame's didFinishLoad (not subframes)

* coalesce calls to determinePrimarySnapshottedPlugInTimerFired from both addPluginView (in case JS is adding lots of plugins in a loop) and didFinishLoad

I'm not sure this is a 100% solution, but it should not regress primary plugin detection while improving performance greatly on some pages (http://thelede.blogs.nytimes.com/2013/04/19/updates-on-aftermath-of-boston-marathon-explosions-2/).

Comment 1 Tim Horton 2013-04-22 15:18:11 PDT
Created attachment 199117 [details]
Comment 2 Tim Horton 2013-04-22 15:18:21 PDT
Going to test more before landing.
Comment 3 Anders Carlsson 2013-04-22 15:31:49 PDT
Comment on attachment 199117 [details]

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

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:3949
> +void WebPage::determinePrimarySnapshottedPlugInTimerFired()
> +{
> +    if (!m_page->settings()->snapshotAllPlugIns() && m_page->settings()->primaryPlugInSnapshotDetectionEnabled())
> +        determinePrimarySnapshottedPlugIn();
> +#endif
> +}

You probably want to put the entire function body inside the #if.
Comment 4 Tim Horton 2013-04-22 15:35:53 PDT