Bug 207042

Summary: [WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default for Cocoa platforms
Product: WebKit Reporter: Antoine Quint <graouts>
Component: New BugsAssignee: Antoine Quint <graouts>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, koivisto, simon.fraser, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=207014
https://bugs.webkit.org/show_bug.cgi?id=112308
Attachments:
Description Flags
Patch zalan: review+

Description Antoine Quint 2020-01-31 07:11:52 PST 
[WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default for Cocoa platforms
Comment 1 Antoine Quint 2020-01-31 07:13:26 PST 
Created attachment 389346 [details]
Patch
Comment 2 Antoine Quint 2020-01-31 07:35:14 PST 
Committed r255494: <https://trac.webkit.org/changeset/255494>
Comment 3 Radar WebKit Bug Importer 2020-01-31 07:36:16 PST 
<rdar://problem/59061483>
Comment 4 Chris Dumez 2020-01-31 08:18:01 PST 
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.
Comment 5 Chris Dumez 2020-01-31 08:18:47 PST 
(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)
Comment 6 Chris Dumez 2020-01-31 08:22:16 PST 
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?
Comment 7 Antoine Quint 2020-01-31 08:34:22 PST 
(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().
Comment 8 Chris Dumez 2020-01-31 08:38:16 PST 
(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!
Comment 9 Antoine Quint 2020-01-31 08:45:12 PST 
(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 :)