Fixed the paint supression to track only render-blocking stylesheets

third_party/WebKit/Source/core/dom/Document.cpp

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index e59fd92e24388367e640533f8a3d9c6eeb3d1eb2..4bf7a60a4557379a78ca37bf42fddd2394f01a5f 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -2290,7 +2290,7 @@ void Document::LayoutUpdated() {
   // of layout thrashing even though that layout might not be followed by
   // a paint for many seconds.
   if (IsRenderingReady() && body() &&
-      !GetStyleEngine().HasPendingScriptBlockingSheets()) {
+      !GetStyleEngine().HasPendingRenderBlockingSheets()) {

[rune, 2017/05/11 19:40:53]

Not sure if I fully wrapped my head around the combination of
HasPendingRenderBlockingSheets and IsRenderingReady() which also rely on the
in-body runtime flag, but this only makes a difference with the flag enabled
which is currently not the case, right?

[Pat Meenan, 2017/05/11 20:25:29]

Good point.  The main issue with IsRenderingReady (and the underlying call to
HaveRenderStylesheetsLoaded()) is that they both have escape hatches for
ignoring pending stylesheets which presumably this is explicitly trying to
bypass (and it also includes a check for imports).

I can create a separate function as part of document
(HasPendingRenderBlockingSheets) that switches based on the flag and checks the
underlying style engine flag without the ability to ignore (though the naming is
still a bit confusing).  Would it make more sense to add a flag to the existing
HaveRenderBlockingStylesheetsLoaded and the underlying style engine functions
that lets the caller explicitly disable the ignore_pending_stylesheets check?

[rune, 2017/05/11 22:06:10]

Thanks for the clarification. Don't you also need to check imports?

If I read the patches correctly - if you have ignore_pending set, render
blocking sheets loaded, but imports not finished, you will MarkFirstLayout. But
loading imports will block rendering, right?
Looking at the existing code, there seems to be some inconsistency, and possibly
a bug there. IsRenderingReady() checks the StyleEngine directly for
ignoring_pending before the calls into the StyleEngine, making the
ignore_pending checks in StyleEngine::Have*Loaded dud.

I'm wondering if getting rid of the ignore_pending check in
StyleEngine::Have*Loaded could be an option?
I suspect (hope) none of the other calls to
HaveScriptBlockingStylesheetsLoaded() than the one in IsRenderingReady() are
called in an ignore_pending context.

[Pat Meenan, 2017/05/12 13:24:08]

I was largely trying to carry-over the existing behavior and just make it handle
the in-body stylesheet case correctly, but yes, now that you mention it, it is
probably buggy with respect to imports.

     if (!document_timing_.FirstLayout())
       document_timing_.MarkFirstLayout();
   }
@@ -2319,7 +2319,7 @@ void Document::ClearFocusedElementTimerFired(TimerBase*) {
 void Document::UpdateStyleAndLayoutTreeIgnorePendingStylesheets() {
   StyleEngine::IgnoringPendingStylesheet ignoring(GetStyleEngine());
 
-  if (GetStyleEngine().HasPendingScriptBlockingSheets()) {
+  if (GetStyleEngine().HasPendingRenderBlockingSheets()) {
     // FIXME: We are willing to attempt to suppress painting with outdated style
     // info only once.  Our assumption is that it would be dangerous to try to
     // stop it a second time, after page content has already been loaded and
@@ -4056,7 +4056,7 @@ void Document::StyleResolverMayHaveChanged() {
   }
 
   if (DidLayoutWithPendingStylesheets() &&
-      !GetStyleEngine().HasPendingScriptBlockingSheets()) {
+      !GetStyleEngine().HasPendingRenderBlockingSheets()) {
     // We need to manually repaint because we avoid doing all repaints in layout
     // or style recalc while sheets are still loading to avoid FOUC.
     pending_sheet_layout_ = kIgnoreLayoutWithPendingSheets;