Bug 114226 - Don't try to remove a non-existent snapshot
Summary: Don't try to remove a non-existent snapshot
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-04-08 17:18 PDT by Dean Jackson
Modified: 2013-04-08 19:42 PDT (History)
6 users (show)

See Also:


Attachments
Patch (2.46 KB, patch)
2013-04-08 17:21 PDT, Dean Jackson
darin: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Build test (with review modifications) (2.61 KB, patch)
2013-04-08 19:34 PDT, Dean Jackson
no flags 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-04-08 17:18:33 PDT
We should not fire the remove snapshot timer if there was never a snapshot to remove.
Comment 1 Radar WebKit Bug Importer 2013-04-08 17:18:57 PDT
<rdar://problem/13604480>
Comment 2 Dean Jackson 2013-04-08 17:21:45 PDT
Created attachment 196975 [details]
Patch
Comment 3 Early Warning System Bot 2013-04-08 17:44:14 PDT
Comment on attachment 196975 [details]
Patch

Attachment 196975 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17672003
Comment 4 Darin Adler 2013-04-08 17:45:33 PDT
Comment on attachment 196975 [details]
Patch

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

> Source/WebCore/html/HTMLPlugInImageElement.cpp:96
> +    DisplayState previousDisplayState = displayState();

This line of code needs to be inside the same #if as the code below; #if PLATFORM(MAC), although I wish that was not the way we guarded this feature! Please fix that before landing.
Comment 5 Early Warning System Bot 2013-04-08 17:55:57 PDT
Comment on attachment 196975 [details]
Patch

Attachment 196975 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17638017
Comment 6 EFL EWS Bot 2013-04-08 18:19:09 PDT
Comment on attachment 196975 [details]
Patch

Attachment 196975 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17641012
Comment 7 Dean Jackson 2013-04-08 19:27:17 PDT
Comment on attachment 196975 [details]
Patch

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

>> Source/WebCore/html/HTMLPlugInImageElement.cpp:96
>> +    DisplayState previousDisplayState = displayState();
> 
> This line of code needs to be inside the same #if as the code below; #if PLATFORM(MAC), although I wish that was not the way we guarded this feature! Please fix that before landing.

It's worse. I can't just move it because we've already updated the state by then. I'll need to put in another #if :(

And yes, this feature guarding is horrible. Since MAC is the only place that is doing the crossfade at the moment, this is an implied connection between WebKit2 and WebCore. I guess it would be better as a Setting.
Comment 8 Dean Jackson 2013-04-08 19:29:57 PDT
Comment on attachment 196975 [details]
Patch

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

>>> Source/WebCore/html/HTMLPlugInImageElement.cpp:96
>>> +    DisplayState previousDisplayState = displayState();
>> 
>> This line of code needs to be inside the same #if as the code below; #if PLATFORM(MAC), although I wish that was not the way we guarded this feature! Please fix that before landing.
> 
> It's worse. I can't just move it because we've already updated the state by then. I'll need to put in another #if :(
> 
> And yes, this feature guarding is horrible. Since MAC is the only place that is doing the crossfade at the moment, this is an implied connection between WebKit2 and WebCore. I guess it would be better as a Setting.

I rewrote the method to but the call to super at the end.
Comment 9 Dean Jackson 2013-04-08 19:34:31 PDT
Created attachment 196981 [details]
Build test (with review modifications)
Comment 10 Dean Jackson 2013-04-08 19:42:28 PDT
Committed r147977: <http://trac.webkit.org/changeset/147977>