Avoid mutating frame owner when detaching a provisional frame.

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

Index: third_party/WebKit/Source/core/frame/Frame.cpp
diff --git a/third_party/WebKit/Source/core/frame/Frame.cpp b/third_party/WebKit/Source/core/frame/Frame.cpp
index 6f7a7402da5898e23d8477e1302556579c2b8883..d259c5d72b7450a82a4d6534d673f801fab881a1 100644
--- a/third_party/WebKit/Source/core/frame/Frame.cpp
+++ b/third_party/WebKit/Source/core/frame/Frame.cpp
@@ -82,7 +82,13 @@ void Frame::detach(FrameDetachType type) {
 
 void Frame::disconnectOwnerElement() {
   if (m_owner) {
-    m_owner->clearContentFrame();
+    // Ocassionally, provisional frames need to be detached, but it shouldn't
+    // affect the frame tree structure. Make sure the frame owner's content
+    // frame actually refers to this frame before clearing it.
+    // TODO(dcheng): https://crbug.com/578349 tracks the cleanup for this once
+    // it's no longer needed.
+    if (m_owner->contentFrame() == this)

[haraken, 2017/01/17 09:30:22]

Is there any way to add something like DCHECK(isProvisionalFrame())?

[dcheng, 2017/01/17 09:38:29]

I considered something like that, but didn't add it in the end for several
reasons:
- The provisional frame state is unique to local frames, so putting it on Frame
is a bit weird.
- If it's LocalFrame only, then disconnectContentFrame() has to be virtual or we
need to downcast to LocalFrame here. That also feels a bit awkward.

In the end, the provisional frame state is supposed to be temporary (i.e. after
PlzNavigate ships, we can clean up the architecture to make this eventually
unnecessary). For that reason, I'm trying to avoid making this concept too
formal for now (since I don't want too much code to depend on it: that's why
there's a TODO in this CL to clean up the random dependencies on
DummyFrameOwner).

WDYT?

+      m_owner->clearContentFrame();
     m_owner = nullptr;
   }
 }