Ensure paint property update and invalidation when a frame owner element changes content view

This is a speculative fix.

The crash reports seem to indicate that an embedded frame which needs
tree_builder_context is walked in PrePaintTreeWalk while the ancestors
didn't create tree_builder_context.

Bug: 974639
Change-Id: Ia37c9b376c1d60b96dae14624cc0a5abf3c0b782
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1673838Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672604}

third_party/blink/renderer/core/html/html_frame_owner_element.cc

@@ -359,7 +359,6 @@ void HTMLFrameOwnerElement::SetEmbeddedContentView(
 
   embedded_content_view_ = embedded_content_view;
   FrameOwnerPropertiesChanged();
-  SetNeedsCompositingUpdate();
 
   GetDocument().GetRootScrollerController().DidUpdateIFrameFrameView(*this);
 
@@ -368,6 +367,8 @@ void HTMLFrameOwnerElement::SetEmbeddedContentView(
   if (!layout_embedded_content)
     return;
 
+  layout_embedded_content->UpdateOnEmbeddedContentViewChange();
+
   if (embedded_content_view_) {
     // TODO(crbug.com/729196): Trace why LocalFrameView::DetachFromLayout
     // crashes.  Perhaps view is getting reattached while document is shutting
@@ -375,7 +376,6 @@ void HTMLFrameOwnerElement::SetEmbeddedContentView(
     if (doc) {
       CHECK_NE(doc->Lifecycle().GetState(), DocumentLifecycle::kStopping);
     }
-    layout_embedded_content->UpdateOnEmbeddedContentViewChange();
 
     DCHECK_EQ(GetDocument().View(), layout_embedded_content->GetFrameView());
     DCHECK(layout_embedded_content->GetFrameView());

third_party/blink/renderer/core/layout/layout_embedded_content.cc

@@ -36,6 +36,7 @@
 #include "third_party/blink/renderer/core/layout/layout_analyzer.h"
 #include "third_party/blink/renderer/core/layout/layout_view.h"
 #include "third_party/blink/renderer/core/paint/embedded_content_painter.h"
+#include "third_party/blink/renderer/core/paint/paint_layer.h"
 
 namespace blink {
 
@@ -317,24 +318,31 @@ PhysicalRect LayoutEmbeddedContent::ReplacedContentRect() const {
 }
 
 void LayoutEmbeddedContent::UpdateOnEmbeddedContentViewChange() {
-  EmbeddedContentView* embedded_content_view = GetEmbeddedContentView();
-  if (!embedded_content_view)
-    return;
-
   if (!Style())
     return;
 
-  if (!NeedsLayout())
-    UpdateGeometry(*embedded_content_view);
+  if (EmbeddedContentView* embedded_content_view = GetEmbeddedContentView()) {
+    if (!NeedsLayout())
+      UpdateGeometry(*embedded_content_view);
 
-  if (StyleRef().Visibility() != EVisibility::kVisible) {
-    embedded_content_view->Hide();
-  } else {
-    embedded_content_view->Show();
-    // FIXME: Why do we issue a full paint invalidation in this case, but not
-    // the other?
-    SetShouldDoFullPaintInvalidation();
+    if (StyleRef().Visibility() != EVisibility::kVisible)
+      embedded_content_view->Hide();
+    else
+      embedded_content_view->Show();
   }
+
+  // One of the reasons of the following is that the layout tree in the new
+  // embedded content view may have already had some paint property and paint
+  // invalidation flags set, and we need to propagate the flags into the host
+  // view. Adding, changing and removing are also significant changes to the
+  // tree so setting the flags ensures the required updates.
+  SetNeedsPaintPropertyUpdate();
+  SetShouldDoFullPaintInvalidation();
+  // Showing/hiding the embedded content view and changing the view between null
+  // and non-null affect compositing (see: PaintLayerCompositor::CanBeComposited
+  // and RootShouldAlwaysComposite).
+  if (HasLayer())
+    Layer()->SetNeedsCompositingInputsUpdate();
 }
 
 void LayoutEmbeddedContent::UpdateGeometry(