Fixed the paint supression to track only render-blocking stylesheets

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All
  * rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
@@ skipping 2272 matching lines @@
 
   // The layout system may perform layouts with pending stylesheets. When
   // recording first layout time, we ignore these layouts, since painting is
   // suppressed for them. We're interested in tracking the time of the
   // first real or 'paintable' layout.
   // TODO(esprehn): This doesn't really make sense, why not track the first
   // beginFrame? This will catch the first layout in a page that does lots
   // 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();
   }
 
   root_scroller_controller_->DidUpdateLayout();
 }
 
 void Document::ClearFocusedElementSoon() {
   if (!clear_focused_element_timer_.IsActive())
     clear_focused_element_timer_.StartOneShot(0, BLINK_FROM_HERE);
 }
 
 void Document::ClearFocusedElementTimerFired(TimerBase*) {
   UpdateStyleAndLayoutTree();
 
   if (focused_element_ && !focused_element_->IsFocusable())
     focused_element_->blur();
 }
 
 // FIXME: This is a bad idea and needs to be removed eventually.
 // Other browsers load stylesheets before they continue parsing the web page.
 // Since we don't, we can run JavaScript code that needs answers before the
 // stylesheets are loaded. Doing a layout ignoring the pending stylesheets
 // lets us get reasonable answers. The long term solution to this problem is
 // to instead suspend JavaScript execution.
 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
     // displayed with accurate style information. (Our suppression involves
     // blanking the whole page at the moment. If it were more refined, we might
     // be able to do something better.) It's worth noting though that this
     // entire method is a hack, since what we really want to do is suspend JS
     // instead of doing a layout with inaccurate information.
     HTMLElement* body_element = body();
     if (body_element && !body_element->GetLayoutObject() &&
@@ skipping 1716 matching lines @@
 }
 
 void Document::StyleResolverMayHaveChanged() {
   if (HasNodesWithPlaceholderStyle()) {
     SetNeedsStyleRecalc(kSubtreeStyleChange,
                         StyleChangeReasonForTracing::Create(
                             StyleChangeReason::kCleanupPlaceholderStyles));
   }
 
   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;
 
     DCHECK(!GetLayoutViewItem().IsNull() || ImportsController());
     if (!GetLayoutViewItem().IsNull())
       GetLayoutViewItem().InvalidatePaintForViewAndCompositedLayers();
   }
 }
 
@@ skipping 2720 matching lines @@
 }
 
 void showLiveDocumentInstances() {
   WeakDocumentSet& set = liveDocumentSet();
   fprintf(stderr, "There are %u documents currently alive:\n", set.size());
   for (blink::Document* document : set)
     fprintf(stderr, "- Document %p URL: %s\n", document,
             document->Url().GetString().Utf8().data());
 }
 #endif