Bug 207042 – [WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default for Cocoa platforms

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Antoine Quint]

[WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default for Cocoa platforms

[Antoine Quint]

Created attachment 389346 [details]
Patch

[Antoine Quint]

Committed r255494: <https://trac.webkit.org/changeset/255494>

[Radar WebKit Bug Importer]

<rdar://problem/59061483>

[Chris Dumez]

Comment on attachment 389346 [details]
Patch

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

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:595
> +        @YES, WebKitHiddenPageCSSAnimationSuspensionEnabledPreferenceKey,

Could you point me to the place where this setting is actually being used? I grep'd it and it looks like dead code to me.

[Chris Dumez]

(In reply to Chris Dumez from comment #4)
> Comment on attachment 389346 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389346&action=review
> 
> > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:595
> > +        @YES, WebKitHiddenPageCSSAnimationSuspensionEnabledPreferenceKey,
> 
> Could you point me to the place where this setting is actually being used? I
> grep'd it and it looks like dead code to me.

Nevermind, found it (bad case sensitivity check)

[Chris Dumez]

Comment on attachment 389346 [details]
Patch

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

>>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:595
>>> +        @YES, WebKitHiddenPageCSSAnimationSuspensionEnabledPreferenceKey,
>> 
>> Could you point me to the place where this setting is actually being used? I grep'd it and it looks like dead code to me.
> 
> Nevermind, found it (bad case sensitivity check)

However, this only does something if Page::setActivityState() gets called (so that Page::setIsVisibleInternal() gets called). I don't see Page::setActivityState() call sites in WebKitLegacy at all, only WebKit2. Am I missing something?

[Antoine Quint]

(In reply to Chris Dumez from comment #6)
> Comment on attachment 389346 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=389346&action=review
> 
> >>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:595
> >>> +        @YES, WebKitHiddenPageCSSAnimationSuspensionEnabledPreferenceKey,
> >> 
> >> Could you point me to the place where this setting is actually being used? I grep'd it and it looks like dead code to me.
> > 
> > Nevermind, found it (bad case sensitivity check)
> 
> However, this only does something if Page::setActivityState() gets called
> (so that Page::setIsVisibleInternal() gets called). I don't see
> Page::setActivityState() call sites in WebKitLegacy at all, only WebKit2. Am
> I missing something?

It's called via Page::setIsVisible().

[Chris Dumez]

(In reply to Antoine Quint from comment #7)
> (In reply to Chris Dumez from comment #6)
> > Comment on attachment 389346 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=389346&action=review
> > 
> > >>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:595
> > >>> +        @YES, WebKitHiddenPageCSSAnimationSuspensionEnabledPreferenceKey,
> > >> 
> > >> Could you point me to the place where this setting is actually being used? I grep'd it and it looks like dead code to me.
> > > 
> > > Nevermind, found it (bad case sensitivity check)
> > 
> > However, this only does something if Page::setActivityState() gets called
> > (so that Page::setIsVisibleInternal() gets called). I don't see
> > Page::setActivityState() call sites in WebKitLegacy at all, only WebKit2. Am
> > I missing something?
> 
> It's called via Page::setIsVisible().

Oh, I missed that. Thanks!

[Antoine Quint]

(In reply to Chris Dumez from comment #8)
> (In reply to Antoine Quint from comment #7)
> > (In reply to Chris Dumez from comment #6)
> > > Comment on attachment 389346 [details]
> > > Patch
> > > 
> > > View in context:
> > > https://bugs.webkit.org/attachment.cgi?id=389346&action=review
> > > 
> > > >>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:595
> > > >>> +        @YES, WebKitHiddenPageCSSAnimationSuspensionEnabledPreferenceKey,
> > > >> 
> > > >> Could you point me to the place where this setting is actually being used? I grep'd it and it looks like dead code to me.
> > > > 
> > > > Nevermind, found it (bad case sensitivity check)
> > > 
> > > However, this only does something if Page::setActivityState() gets called
> > > (so that Page::setIsVisibleInternal() gets called). I don't see
> > > Page::setActivityState() call sites in WebKitLegacy at all, only WebKit2. Am
> > > I missing something?
> > 
> > It's called via Page::setIsVisible().
> 
> Oh, I missed that. Thanks!

Easy when you've been staring at stack traces all day :)