Bug 114226 – Don't try to remove a non-existent snapshot

• Status: RESOLVED
• Resolution: FIXED
• Component: Plug-ins

[Dean Jackson]

We should not fire the remove snapshot timer if there was never a snapshot to remove.

[Radar WebKit Bug Importer]

<rdar://problem/13604480>

[Dean Jackson]

Created attachment 196975 [details]
Patch

[Early Warning System Bot]

Comment on attachment 196975 [details]
Patch

Attachment 196975 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17672003

[Darin Adler]

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.

[Early Warning System Bot]

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

[EFL EWS Bot]

Comment on attachment 196975 [details]
Patch

Attachment 196975 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17641012

[Dean Jackson]

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.

[Dean Jackson]

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.

[Dean Jackson]

Created attachment 196981 [details]
Build test (with review modifications)

[Dean Jackson]

Committed r147977: <http://trac.webkit.org/changeset/147977>