Summary: | Pages using Google's anti-flicker optimization may take ~5 seconds to do initial paint | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
Component: | CSS | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, commit-queue, ggaren, mcatanzaro, rniwa, saam, webkit-bug-importer, zalan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Bug Depends on: | 199704 | ||||||
Bug Blocks: | |||||||
Attachments: |
|
Description
Chris Dumez
2019-06-24 13:26:23 PDT
Created attachment 372794 [details]
Patch
Comment on attachment 372794 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=372794&action=review r=me > Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:240 > + if (currentDocument->settings().googleAntiFlickerOptimizationQuirkEnabled() && url == "https://www.google-analytics.com/analytics.js"_str) { Should we also check site-specific quirks enabled here? Would be nice to enable a developer to test a fix in this area with our quirk turned off. (In reply to Geoffrey Garen from comment #3) > Comment on attachment 372794 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372794&action=review > > r=me > > > Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:240 > > + if (currentDocument->settings().googleAntiFlickerOptimizationQuirkEnabled() && url == "https://www.google-analytics.com/analytics.js"_str) { > > Should we also check site-specific quirks enabled here? Would be nice to > enable a developer to test a fix in this area with our quirk turned off. Did we fix the fact that site-specific quirks are enabled on macOS or iOS (do not remember which one of the 2). This applies to both platforms, which is why I did not do that. (In reply to Geoffrey Garen from comment #3) > Comment on attachment 372794 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=372794&action=review > > r=me > > > Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:240 > > + if (currentDocument->settings().googleAntiFlickerOptimizationQuirkEnabled() && url == "https://www.google-analytics.com/analytics.js"_str) { > > Should we also check site-specific quirks enabled here? Would be nice to > enable a developer to test a fix in this area with our quirk turned off. They can test that using the experimental feature flag. > Did we fix the fact that site-specific quirks are enabled on macOS or iOS
> (do not remember which one of the 2). This applies to both platforms, which
> is why I did not do that.
I *think* we did, but I didn't confirm. Anyway, you're right that the experimental feature flag is enough.
Comment on attachment 372794 [details] Patch Clearing flags on attachment: 372794 Committed r246764: <https://trac.webkit.org/changeset/246764> All reviewed patches have been landed. Closing bug. I guess you wanted it inside the results.summary.blockedLoad condition: [2134/3055] Building CXX object Source/WebCore/CMakeFiles...es/WebCore/unified-sources/UnifiedSource-5037b3e8-2.cpp.o In file included from DerivedSources/WebCore/unified-sources/UnifiedSource-5037b3e8-2.cpp:2: ../../Source/WebCore/contentextensions/ContentExtensionsBackend.cpp: In member function ‘WebCore::ContentRuleListResults WebCore::ContentExtensions::ContentExtensionsBackend::processContentRuleListsForLoad(const WTF::URL&, WTF::OptionSet<WebCore::ContentExtensions::ResourceType>, WebCore::DocumentLoader&)’: ../../Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:235:9: warning: this ‘if’ clause does not guard... [-Wmisleading-indentation] 235 | if (results.summary.blockedLoad) | ^~ ../../Source/WebCore/contentextensions/ContentExtensionsBackend.cpp:240:13: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the ‘if’ 240 | if (currentDocument->settings().googleAntiFlickerOptimizationQuirkEnabled() && url == "https://www.google-analytics.com/analytics.js"_str) { | ^~ Committed r247186: <https://trac.webkit.org/changeset/247186> |