Bug 199173 – Pages using Google's anti-flicker optimization may take ~5 seconds to do initial paint

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Chris Dumez]

Pages using Google's anti-flicker optimization [1] take ~5 seconds to do initial paint when analytics.js load is blocked by a content blocker.

[1] https://developers.google.com/optimize/

[Chris Dumez]

<rdar://problem/45968770>

[Chris Dumez]

Created attachment 372794 [details]
Patch

[Geoffrey Garen]

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.

[Chris Dumez]

(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.

[Chris Dumez]

(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.

[Geoffrey Garen]

> 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.

[WebKit Commit Bot]

Comment on attachment 372794 [details]
Patch

Clearing flags on attachment: 372794

Committed r246764: <https://trac.webkit.org/changeset/246764>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Michael Catanzaro]

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) {
      |             ^~

[Michael Catanzaro]

Committed r247186: <https://trac.webkit.org/changeset/247186>