[PaintWorklet] Fix crash when hitting breakpoint during lifecycle

If we put a devtool's breakpoint in the paint() function and start
debugging, the renderer will crash when we resize the window or switch
to the element tab. This is because when we are in the middle of
lifecycle when we get to the paint function, and anything triggers
layout update will cause a renderer crash.

The fix in CL is that when we hit a breakpoint, check if we are in the
middle of lifecycle or not, if we are, flip a bit to be true in the
|LocalFrameView|, so that later on whenever there is a layout update we
can check that bit and do early exit.

Bug: 788219
Change-Id: I4fbafab868508ed4ba453339d91364253d17f523
Reviewed-on: https://chromium-review.googlesource.com/899484
Commit-Queue: Xida Chen <xidachen@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537004}

third_party/WebKit/Source/core/BUILD.gn

@@ -1898,6 +1898,7 @@ jumbo_source_set("unit_tests") {
     "input/ScrollSnapTest.cpp",
     "input/TouchActionTest.cpp",
     "input/TouchEventManagerTest.cpp",
+    "inspector/MainThreadDebuggerTest.cpp",
     "inspector/ProtocolParserTest.cpp",
     "intersection_observer/IntersectionObserverTest.cpp",
     "layout/CollapsedBorderValueTest.cpp",

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

@@ -2108,6 +2108,8 @@ static void AssertLayoutTreeUpdated(Node& root) {
 
 void Document::UpdateStyleAndLayoutTree() {
   DCHECK(IsMainThread());
+  if (Lifecycle().LifecyclePostponed())
+    return;
 
   HTMLFrameOwnerElement::PluginDisposeSuspendScope suspend_plugin_dispose;
   ScriptForbiddenScope forbid_script;
@@ -2434,6 +2436,8 @@ void Document::ClearFocusedElementTimerFired(TimerBase*) {
 // lets us get reasonable answers. The long term solution to this problem is
 // to instead suspend JavaScript execution.
 void Document::UpdateStyleAndLayoutTreeIgnorePendingStylesheets() {
+  if (Lifecycle().LifecyclePostponed())
+    return;
   // See comment for equivalent CHECK in Document::UpdateStyleAndLayoutTree.
   // Updating style and layout can dirty state that must remain clean during
   // lifecycle updates.

third_party/WebKit/Source/core/dom/DocumentLifecycle.h

@@ -178,6 +178,29 @@ class CORE_EXPORT DocumentLifecycle {
     DISALLOW_COPY_AND_ASSIGN(DisallowThrottlingScope);
   };
 
+  // If we hit a devtool break point in the middle of document lifecycle, for
+  // example, https://crbug.com/788219, this scope is triggered and no more
+  // layout or style computation is allowed.
+  // This class should never be used outside of debugging.
+  class PostponeTransitionScope {
+   public:
+    explicit PostponeTransitionScope(DocumentLifecycle& document_lifecycle)
+        : document_lifecycle_(document_lifecycle) {
+      document_lifecycle_.SetLifecyclePostponed();
+    }
+    ~PostponeTransitionScope() {}
+
+    void SetLifecyclePostponed() {
+      document_lifecycle_.SetLifecyclePostponed();
+    }
+    void ResetLifecyclePostponed() {
+      document_lifecycle_.ResetLifecyclePostponed();
+    }
+
+   private:
+    DocumentLifecycle& document_lifecycle_;
+  };
+
   DocumentLifecycle();
   ~DocumentLifecycle();
 
@@ -208,19 +231,25 @@ class CORE_EXPORT DocumentLifecycle {
   }
 
   bool ThrottlingAllowed() const;
+  bool LifecyclePostponed() const { return life_cycle_postponed_; }
 
 #if DCHECK_IS_ON()
   WTF::String ToString() const;
 #endif
  private:
+  friend class PostponeTransitionScope;
 #if DCHECK_IS_ON()
   bool CanAdvanceTo(LifecycleState) const;
   bool CanRewindTo(LifecycleState) const;
 #endif
 
+  void SetLifecyclePostponed() { life_cycle_postponed_ = true; }
+  void ResetLifecyclePostponed() { life_cycle_postponed_ = false; }
+
   LifecycleState state_;
   int detach_count_;
   int disallow_transition_count_;
+  bool life_cycle_postponed_;
   DISALLOW_COPY_AND_ASSIGN(DocumentLifecycle);
 };
 

third_party/WebKit/Source/core/frame/LocalFrameView.cpp

@@ -1598,6 +1598,9 @@ void LocalFrameView::ViewportSizeChanged(bool width_changed,
                                          bool height_changed) {
   DCHECK(width_changed || height_changed);
   DCHECK(frame_->GetPage());
+  if (frame_->GetDocument() &&
+      frame_->GetDocument()->Lifecycle().LifecyclePostponed())
+    return;
 
   bool root_layer_scrolling_enabled =
       RuntimeEnabledFeatures::RootLayerScrollingEnabled();
@@ -2026,6 +2029,9 @@ IntSize LocalFrameView::GetLayoutSize(
 
 void LocalFrameView::SetLayoutSize(const IntSize& size) {
   DCHECK(!LayoutSizeFixedToFrameSize());
+  if (frame_->GetDocument() &&
+      frame_->GetDocument()->Lifecycle().LifecyclePostponed())
+    return;
 
   SetLayoutSizeInternal(size);
 }
@@ -3126,6 +3132,9 @@ void LocalFrameView::ClearPrintContext() {
 // WebPluginContainerImpls.
 bool LocalFrameView::UpdateLifecyclePhasesInternal(
     DocumentLifecycle::LifecycleState target_state) {
+  if (frame_->GetDocument() &&
+      frame_->GetDocument()->Lifecycle().LifecyclePostponed())
+    return false;
   if (current_update_lifecycle_phases_target_state_ !=
       DocumentLifecycle::kUninitialized) {
     NOTREACHED()

third_party/WebKit/Source/core/inspector/InspectorLayerTreeAgent.cpp

@@ -434,6 +434,18 @@ Response InspectorLayerTreeAgent::makeSnapshot(const String& layer_id,
   IntRect interest_rect(IntPoint(0, 0), size);
   suppress_layer_paint_events_ = true;
 
+  // If we hit a devtool break point in the middle of document lifecycle, for
+  // example, https://crbug.com/788219, this will prevent crash when clicking
+  // the "layer" panel.
+  if (inspected_frames_->Root()->View()->GetFrame().GetDocument() &&
+      inspected_frames_->Root()
+          ->View()
+          ->GetFrame()
+          .GetDocument()
+          ->Lifecycle()
+          .LifecyclePostponed())
+    return Response::Error("Layer does not draw content");
+
   inspected_frames_->Root()->View()->UpdateAllLifecyclePhasesExceptPaint();
   for (auto frame = inspected_frames_->begin();
        frame != inspected_frames_->end(); ++frame) {

third_party/WebKit/Source/core/inspector/MainThreadDebugger.cpp

@@ -231,17 +231,39 @@ void MainThreadDebugger::InterruptMainThreadAndRun(
   }
 }
 
+// In the test, we just assume that we hit a devtool's break point during the
+// lifecycle.
+void MainThreadDebugger::SetPostponeTransitionScopeForTesting(
+    Document& document) {
+  if (postponed_transition_scope_) {
+    postponed_transition_scope_->SetLifecyclePostponed();
+  } else {
+    postponed_transition_scope_ =
+        std::make_unique<DocumentLifecycle::PostponeTransitionScope>(
+            document.Lifecycle());
+  }
+}
+
 void MainThreadDebugger::runMessageLoopOnPause(int context_group_id) {
   LocalFrame* paused_frame =
       WeakIdentifierMap<LocalFrame>::Lookup(context_group_id);
   // Do not pause in Context of detached frame.
   if (!paused_frame)
     return;
-  // TODO(crbug.com/788219): this is a temporary hack that disables breakpoint
-  // for paint worklet.
+  // If we hit a break point in the paint() function for CSS paint, then we are
+  // in the middle of document life cycle. In this case, we should not allow
+  // any style update or layout, which could be triggered by resizing the
+  // browser window, or clicking at the element panel on devtool.
   if (paused_frame->GetDocument() &&
-      !paused_frame->GetDocument()->Lifecycle().StateAllowsTreeMutations())
-    return;
+      !paused_frame->GetDocument()->Lifecycle().StateAllowsTreeMutations()) {
+    if (postponed_transition_scope_) {
+      postponed_transition_scope_->SetLifecyclePostponed();
+    } else {
+      postponed_transition_scope_ =
+          std::make_unique<DocumentLifecycle::PostponeTransitionScope>(
+              paused_frame->GetDocument()->Lifecycle());
+    }
+  }
   DCHECK(paused_frame == paused_frame->LocalFrameRoot());
   paused_ = true;
 
@@ -252,8 +274,15 @@ void MainThreadDebugger::runMessageLoopOnPause(int context_group_id) {
     client_message_loop_->Run(paused_frame);
 }
 
+void MainThreadDebugger::ResetPostponeTransitionScopeForTesting() {
+  if (postponed_transition_scope_)
+    postponed_transition_scope_->ResetLifecyclePostponed();
+}
+
 void MainThreadDebugger::quitMessageLoopOnPause() {
   paused_ = false;
+  if (postponed_transition_scope_)
+    postponed_transition_scope_->ResetLifecyclePostponed();
   if (client_message_loop_)
     client_message_loop_->QuitNow();
 }

third_party/WebKit/Source/core/inspector/MainThreadDebugger.h

@@ -47,6 +47,7 @@ class ErrorEvent;
 class LocalFrame;
 class SecurityOrigin;
 class SourceLocation;
+class DocumentLifecycle;
 
 class CORE_EXPORT MainThreadDebugger final : public ThreadDebugger {
  public:
@@ -80,6 +81,9 @@ class CORE_EXPORT MainThreadDebugger final : public ThreadDebugger {
   void ContextWillBeDestroyed(ScriptState*);
   void ExceptionThrown(ExecutionContext*, ErrorEvent*);
 
+  void SetPostponeTransitionScopeForTesting(Document&);
+  void ResetPostponeTransitionScopeForTesting();
+
  private:
   void ReportConsoleMessage(ExecutionContext*,
                             MessageSource,
@@ -121,6 +125,8 @@ class CORE_EXPORT MainThreadDebugger final : public ThreadDebugger {
   std::unique_ptr<InspectorTaskRunner> task_runner_;
   bool paused_;
   static MainThreadDebugger* instance_;
+  std::unique_ptr<DocumentLifecycle::PostponeTransitionScope>
+      postponed_transition_scope_;
   DISALLOW_COPY_AND_ASSIGN(MainThreadDebugger);
 };
 

third_party/WebKit/Source/core/inspector/MainThreadDebuggerTest.cpp

+// Copyright 2018 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/inspector/MainThreadDebugger.h"
+
+#include <memory>
+#include "core/testing/PageTestBase.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+class MainThreadDebuggerTest : public PageTestBase {
+ public:
+  MainThreadDebugger* GetMainThreadDebugger() {
+    return MainThreadDebugger::Instance();
+  }
+
+ private:
+};
+
+TEST_F(MainThreadDebuggerTest, HitBreakPointDuringLifecycle) {
+  MainThreadDebugger* debugger = GetMainThreadDebugger();
+  Document& document = GetDocument();
+  debugger->SetPostponeTransitionScopeForTesting(document);
+  EXPECT_TRUE(document.Lifecycle().LifecyclePostponed());
+
+  // The following steps would cause either style update or layout, it should
+  // never crash.
+  document.View()->ViewportSizeChanged(true, true);
+  document.View()->UpdateAllLifecyclePhases();
+  document.UpdateStyleAndLayoutIgnorePendingStylesheets();
+  document.UpdateStyleAndLayoutTree();
+
+  debugger->ResetPostponeTransitionScopeForTesting();
+  EXPECT_FALSE(document.Lifecycle().LifecyclePostponed());
+}
+
+}  // namespace blink