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+

Antoine Quint
Reported 2020-01-31 07:11:52 PST
[WK1] hiddenPageCSSAnimationSuspensionEnabled should be enabled by default for Cocoa platforms
Attachments
Patch (2.07 KB, patch)
2020-01-31 07:13 PST, Antoine Quint 		zalan: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Antoine Quint
Comment 1 2020-01-31 07:13:26 PST
Created attachment 389346 [details] Patch
Antoine Quint
Comment 2 2020-01-31 07:35:14 PST
Committed r255494: <https://trac.webkit.org/changeset/255494>
Radar WebKit Bug Importer
Comment 3 2020-01-31 07:36:16 PST
<rdar://problem/59061483>
Chris Dumez
Comment 4 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.
Chris Dumez
Comment 5 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)
Chris Dumez
Comment 6 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?
Antoine Quint
Comment 7 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().
Chris Dumez
Comment 8 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!
Antoine Quint
Comment 9 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 :)
Note You need to log in before you can comment on or make changes to this bug.