Bug 207042 - [WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default for Cocoa platforms
Summary: [WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default fo...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Antoine Quint
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-31 07:11 PST by Antoine Quint
Modified: 2020-01-31 08:45 PST (History)
5 users (show)

See Also:


Attachments
Patch (2.07 KB, patch)
2020-01-31 07:13 PST, Antoine Quint
zalan: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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 :)