Bug 111975 - Plugins created during user gestures (or soon after) should not be snapshotted
Summary: Plugins created during user gestures (or soon after) should not be snapshotted
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dean Jackson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-03-11 04:56 PDT by Dean Jackson
Modified: 2013-03-12 11:12 PDT (History)
7 users (show)

See Also:


Attachments
Patch (8.50 KB, patch)
2013-03-11 07:42 PDT, Dean Jackson
no flags Details | Formatted Diff | Diff
Patch (8.76 KB, patch)
2013-03-11 14:22 PDT, Dean Jackson
thorton: review+
peter+ews: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dean Jackson 2013-03-11 04:56:21 PDT
There are many sites which create plugins in response to user actions, such as clicking on a poster frame. We should make sure that we never snapshot in that case. It's not quite so simple though, because sometimes the user action inserts an iframe that loads the plugin, meaning the action and the plugin creation are separate events. The suggested solution is to "bless" all plugins that were created within a few seconds of a user gesture.
Comment 1 Dean Jackson 2013-03-11 04:56:46 PDT
<rdar://problem/13232172>
Comment 2 Dean Jackson 2013-03-11 07:42:52 PDT
Created attachment 192468 [details]
Patch
Comment 3 WebKit Review Bot 2013-03-11 08:08:28 PDT
Comment on attachment 192468 [details]
Patch

Attachment 192468 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17134207
Comment 4 WebKit Review Bot 2013-03-11 08:20:55 PDT
Comment on attachment 192468 [details]
Patch

Attachment 192468 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17149143
Comment 5 Peter Beverloo (cr-android ews) 2013-03-11 08:28:55 PDT
Comment on attachment 192468 [details]
Patch

Attachment 192468 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17169149
Comment 6 Dean Jackson 2013-03-11 14:22:21 PDT
Created attachment 192559 [details]
Patch
Comment 7 Peter Beverloo (cr-android ews) 2013-03-11 15:44:43 PDT
Comment on attachment 192559 [details]
Patch

Attachment 192559 [details] did not pass cr-android-ews (chromium-android):
Output: http://webkit-commit-queue.appspot.com/results/17177331
Comment 8 Dean Jackson 2013-03-11 15:54:52 PDT
Committed r145421: <http://trac.webkit.org/changeset/145421>
Comment 9 James Robinson 2013-03-11 16:23:41 PDT
(In reply to comment #8)
> Committed r145421: <http://trac.webkit.org/changeset/145421>

This doesn't compile on any chromium port (and probably others) since it adds a reference to currentTime() without #include'ing <wtf/CurrentTime.h>.  The EWS bots all pointed out this compile failure - can I ask why you committed anyway without addressing it?
Comment 10 Tim Horton 2013-03-11 16:26:09 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Committed r145421: <http://trac.webkit.org/changeset/145421>
> 
> This doesn't compile on any chromium port (and probably others) since it adds a reference to currentTime() without #include'ing <wtf/CurrentTime.h>.  The EWS bots all pointed out this compile failure - can I ask why you committed anyway without addressing it?

I'm going to guess he thought that the delta between the two patches would fix the build failures on chromium (it didn't). Even now, the second patch hasn't made it through any of the chromium EWS bots, and is green/yellow across the board.
Comment 11 Dean Jackson 2013-03-12 11:12:11 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > Committed r145421: <http://trac.webkit.org/changeset/145421>
> 
> This doesn't compile on any chromium port (and probably others) since it adds a reference to currentTime() without #include'ing <wtf/CurrentTime.h>.  The EWS bots all pointed out this compile failure - can I ask why you committed anyway without addressing it?

Sorry, I screwed up badly. As Tim suggested, I thought that the issue was it being included from somewhere that hadn't defined currentTime, but I was too stupid to check that it was included when I moved it to the cpp file :(