Bug 1699768 - Don't unsuppress painting until we've known the website background, to prevent flashing. r=smaug
☠☠ backed out by c4571cca3832 ☠ ☠
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 23 Mar 2021 10:16:52 +0000
changeset 572583 1d4a266e0e49dc7bd12b9bfbf30ec4c40dd6464d
parent 572582 8452d5b6277877ca9f7e469a10247689c32824fa
child 572584 43ae915dc91a222a7eda8916b7982f2749a223bd
push id139243
push userealvarez@mozilla.com
push dateTue, 23 Mar 2021 10:19:16 +0000
treeherderautoland@1d4a266e0e49 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1699768
milestone89.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1699768 - Don't unsuppress painting until we've known the website background, to prevent flashing. r=smaug Actually the page in this case starts getting styled _after_ the load event, sometimes, but when that happens that also causes a white flash in other browsers. Differential Revision: https://phabricator.services.mozilla.com/D109392
layout/base/PresShell.cpp
layout/base/PresShell.h
layout/base/PresShellForwards.h
modules/libpref/init/StaticPrefList.yaml
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -1340,20 +1340,17 @@ void PresShell::Destroy() {
         os->RemoveObserver(this, "sessionstore-one-or-no-tab-restored");
       }
       os->RemoveObserver(this, "font-info-updated");
       os->RemoveObserver(this, "look-and-feel-changed");
     }
   }
 
   // If our paint suppression timer is still active, kill it.
-  if (mPaintSuppressionTimer) {
-    mPaintSuppressionTimer->Cancel();
-    mPaintSuppressionTimer = nullptr;
-  }
+  CancelPaintSuppressionTimer();
 
   // Same for our reflow continuation timer
   if (mReflowContinueTimer) {
     mReflowContinueTimer->Cancel();
     mReflowContinueTimer = nullptr;
   }
 
   if (mDelayedPaintTimer) {
@@ -1812,16 +1809,39 @@ void PresShell::EndObservingDocument() {
   mIsDocumentGone = true;
   mIsObservingDocument = false;
 }
 
 #ifdef DEBUG_kipp
 char* nsPresShell_ReflowStackPointerTop;
 #endif
 
+void PresShell::InitPaintSuppressionTimer() {
+  // Default to PAINTLOCK_EVENT_DELAY if we can't get the pref value.
+  Document* doc = mDocument->GetDisplayDocument()
+                      ? mDocument->GetDisplayDocument()
+                      : mDocument.get();
+  const bool inProcess = !doc->GetBrowsingContext() ||
+                         doc->GetBrowsingContext()->Top()->IsInProcess();
+  int32_t delay = inProcess
+                      ? StaticPrefs::nglayout_initialpaint_delay()
+                      : StaticPrefs::nglayout_initialpaint_delay_in_oopif();
+  if (mPaintSuppressionAttempts) {
+    delay += mPaintSuppressionAttempts *
+             StaticPrefs::nglayout_initialpaint_retry_extra_delay();
+  }
+  mPaintSuppressionTimer->InitWithNamedFuncCallback(
+      [](nsITimer* aTimer, void* aPresShell) {
+        RefPtr<PresShell> self = static_cast<PresShell*>(aPresShell);
+        self->UnsuppressPaintingFromTimer();
+      },
+      this, delay, nsITimer::TYPE_ONE_SHOT,
+      "PresShell::sPaintSuppressionCallback");
+}
+
 nsresult PresShell::Initialize() {
   if (mIsDestroying) {
     return NS_OK;
   }
 
   if (!mDocument) {
     // Nothing to do
     return NS_OK;
@@ -1927,50 +1947,31 @@ nsresult PresShell::Initialize() {
     Document::ReadyState readyState = mDocument->GetReadyStateEnum();
     if (readyState != Document::READYSTATE_COMPLETE) {
       mPaintSuppressionTimer = NS_NewTimer();
     }
     if (!mPaintSuppressionTimer) {
       mPaintingSuppressed = false;
     } else {
       // Initialize the timer.
-
-      // Default to PAINTLOCK_EVENT_DELAY if we can't get the pref value.
-      Document* doc = mDocument->GetDisplayDocument()
-                          ? mDocument->GetDisplayDocument()
-                          : mDocument.get();
-      int32_t delay = Preferences::GetInt(
-          !doc->GetBrowsingContext() ||
-                  doc->GetBrowsingContext()->Top()->IsInProcess()
-              ? "nglayout.initialpaint.delay"
-              : "nglayout.initialpaint.delay_in_oopif",
-          PAINTLOCK_EVENT_DELAY);
-
       mPaintSuppressionTimer->SetTarget(
           mDocument->EventTargetFor(TaskCategory::Other));
-      mPaintSuppressionTimer->InitWithNamedFuncCallback(
-          sPaintSuppressionCallback, this, delay, nsITimer::TYPE_ONE_SHOT,
-          "PresShell::sPaintSuppressionCallback");
+      InitPaintSuppressionTimer();
     }
   }
 
   // If we get here and painting is not suppressed, we still want to run the
   // unsuppression logic, so set mShouldUnsuppressPainting to true.
   if (!mPaintingSuppressed) {
     mShouldUnsuppressPainting = true;
   }
 
   return NS_OK;  // XXX this needs to be real. MMP
 }
 
-void PresShell::sPaintSuppressionCallback(nsITimer* aTimer, void* aPresShell) {
-  RefPtr<PresShell> self = static_cast<PresShell*>(aPresShell);
-  if (self) self->UnsuppressPainting();
-}
-
 nsresult PresShell::ResizeReflow(nscoord aWidth, nscoord aHeight,
                                  ResizeReflowOptions aOptions) {
   if (mZoomConstraintsClient) {
     // If we have a ZoomConstraintsClient and the available screen area
     // changed, then we might need to disable double-tap-to-zoom, so notify
     // the ZCC to update itself.
     mZoomConstraintsClient->ScreenSizeChanged();
   }
@@ -3896,31 +3897,61 @@ void PresShell::UnsuppressAndInvalidate(
   mPaintingSuppressed = false;
   nsIFrame* rootFrame = mFrameConstructor->GetRootFrame();
   if (rootFrame) {
     // let's assume that outline on a root frame is not supported
     rootFrame->InvalidateFrame();
   }
 
   // now that painting is unsuppressed, focus may be set on the document
-  if (nsPIDOMWindowOuter* win = mDocument->GetWindow()) win->SetReadyForFocus();
+  if (nsPIDOMWindowOuter* win = mDocument->GetWindow()) {
+    win->SetReadyForFocus();
+  }
 
   if (!mHaveShutDown) {
     SynthesizeMouseMove(false);
     ScheduleApproximateFrameVisibilityUpdateNow();
   }
 }
 
-void PresShell::UnsuppressPainting() {
+void PresShell::CancelPaintSuppressionTimer() {
   if (mPaintSuppressionTimer) {
     mPaintSuppressionTimer->Cancel();
     mPaintSuppressionTimer = nullptr;
   }
-
-  if (mIsDocumentGone || !mPaintingSuppressed) return;
+}
+
+void PresShell::UnsuppressPaintingFromTimer() {
+  if (mIsDocumentGone || !mPaintingSuppressed) {
+    CancelPaintSuppressionTimer();
+    return;
+  }
+  if (!StaticPrefs::nglayout_initialpaint_unsuppress_with_no_background()) {
+    if (mPresContext->IsRootContentDocumentCrossProcess()) {
+      UpdateCanvasBackground();
+      if (!mHasCSSBackgroundColor) {
+        // We don't unsuppress painting if the page has a transparent
+        // background, as that usually means that the page is unstyled.
+        if (mPaintSuppressionAttempts++ <
+            StaticPrefs::nglayout_initialpaint_retry_max_retry_count()) {
+          InitPaintSuppressionTimer();
+          return;
+        }
+      }
+    }
+  }
+  UnsuppressPainting();
+}
+
+void PresShell::UnsuppressPainting() {
+  CancelPaintSuppressionTimer();
+
+  if (mIsDocumentGone || !mPaintingSuppressed) {
+    return;
+  }
 
   // If we have reflows pending, just wait until we process
   // the reflows and get all the frames where we want them
   // before actually unlocking the painting.  Otherwise
   // go ahead and unlock now.
   if (!mDirtyRoots.IsEmpty())
     mShouldUnsuppressPainting = true;
   else
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -674,20 +674,20 @@ class PresShell final : public nsStubDoc
 
   /**
    * Called to find out if painting is suppressed for this presshell.  If it is
    * suppressd, we don't allow the painting of any layer but the background, and
    * we don't recur into our children.
    */
   bool IsPaintingSuppressed() const { return mPaintingSuppressed; }
 
-  /**
-   * Unsuppress painting.
-   */
   void UnsuppressPainting();
+  void UnsuppressPaintingFromTimer();
+  void InitPaintSuppressionTimer();
+  void CancelPaintSuppressionTimer();
 
   /**
    * Reconstruct frames for all elements in the document
    */
   MOZ_CAN_RUN_SCRIPT void ReconstructFrames();
 
   /**
    * See if reflow verification is enabled. To enable reflow verification add
@@ -2715,19 +2715,16 @@ class PresShell final : public nsStubDoc
     static TimeStamp sLastInputProcessed;
     static StaticRefPtr<dom::Element> sLastKeyDownEventTargetElement;
   };
 
   PresShell* GetRootPresShell() const;
 
   nscolor GetDefaultBackgroundColorToDraw();
 
-  // The callback for the mPaintSuppressionTimer timer.
-  static void sPaintSuppressionCallback(nsITimer* aTimer, void* aPresShell);
-
   //////////////////////////////////////////////////////////////////////////////
   // Approximate frame visibility tracking implementation.
   //////////////////////////////////////////////////////////////////////////////
 
   void UpdateApproximateFrameVisibility();
   void DoUpdateApproximateFrameVisibility(bool aRemoveOnly);
 
   void ClearApproximatelyVisibleFramesList(
@@ -2987,16 +2984,19 @@ class PresShell final : public nsStubDoc
 
   // Flags controlling how our document is rendered.  These persist
   // between paints and so are tied with retained layer pixels.
   // PresShell flushes retained layers when the rendering state
   // changes in a way that prevents us from being able to (usefully)
   // re-use old pixels.
   RenderingStateFlags mRenderingStateFlags;
 
+  // The amount of times that we have tried to unsuppress painting off a timer.
+  int32_t mPaintSuppressionAttempts = 0;
+
   // Whether we're currently under a FlushPendingNotifications.
   // This is used to handle flush reentry correctly.
   // NOTE: This can't be a bitfield since AutoRestore has a reference to this
   // variable.
   bool mInFlush;
 
   bool mCaretEnabled : 1;
 
--- a/layout/base/PresShellForwards.h
+++ b/layout/base/PresShellForwards.h
@@ -41,20 +41,16 @@ enum class ResizeReflowOptions : uint32_
   // TODO(emilio): Ideally this should just become the default, or we should
   // unconditionally not reflow and rely on the caller to do so, having a
   // separate API for shrink-to-fit.
   SuppressReflow = 1 << 1,
 };
 
 MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ResizeReflowOptions)
 
-// This is actually pref-controlled, but we use this value if we fail to get
-// the pref for any reason.
-#define PAINTLOCK_EVENT_DELAY 5
-
 enum class IntrinsicDirty {
   // XXXldb eResize should be renamed
   Resize,       // don't mark any intrinsic widths dirty
   TreeChange,   // mark intrinsic widths dirty on aFrame and its ancestors
   StyleChange,  // Do eTreeChange, plus all of aFrame's descendants
 };
 
 enum class ReflowRootHandling {
--- a/modules/libpref/init/StaticPrefList.yaml
+++ b/modules/libpref/init/StaticPrefList.yaml
@@ -9240,16 +9240,51 @@
 
 # Enable/disable widget update area flashing --- only supported with
 # BasicLayers (other layer managers always update the entire widget area).
 - name: nglayout.debug.widget_update_flashing
   type: RelaxedAtomicBool
   value: false
   mirror: always
 
+- name: nglayout.initialpaint.delay
+  type: int32_t
+  value: 5
+  mirror: always
+
+- name: nglayout.initialpaint.delay_in_oopif
+  type: int32_t
+  value: 5
+  mirror: always
+
+# Whether we should unsuppress painting from the initial paint timer when
+# there's no CSS background specified on the page.
+- name: nglayout.initialpaint.unsuppress_with_no_background
+  type: bool
+  value: false
+  mirror: always
+
+# If we retried, what is the extra delay we should use.
+# Note that this increment is multiplied for the current retry number.
+- name: nglayout.initialpaint.retry.extra_delay
+  type: int32_t
+  # Somewhat arbitrary, but not too high so as to miss lots of frames if the
+  # page begins to get styled. Note that we forcefully unsuppress when the load
+  # event arrives too, or on max retries, so this won't block painting if the
+  # page has loaded.
+  value: 25
+  mirror: always
+
+- name: nglayout.initialpaint.retry.max_retry_count
+  type: int32_t
+  # This gives us 5 + 30 + 55 + ... 380 == 2705ms before forcibly
+  # unsuppressing, which seems plenty.
+  value: 15
+  mirror: always
+
 #---------------------------------------------------------------------------
 # Prefs starting with "page_load."
 #---------------------------------------------------------------------------
 
 # Time in milliseconds during which certain tasks are deprioritized during
 # page load.
 - name: page_load.deprioritization_period
   type: RelaxedAtomicUint32