Avoid mutating frame owner when detaching a provisional frame.

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

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Simon Hausmann <hausmann@kde.org>
  *                     2000 Stefan Schimanski <1Stein@gmx.de>
  *                     2001 George Staikos <staikos@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All
  * rights reserved.
  * Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com>
@@ skipping 64 matching lines @@
   disconnectOwnerElement();
   // After this, we must no longer talk to the client since this clears
   // its owning reference back to our owning LocalFrame.
   m_client->detached(type);
   m_client = nullptr;
   m_host = nullptr;
 }
 
 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;
   }
 }
 
 Page* Frame::page() const {
   if (m_host)
     return &m_host->page();
   return nullptr;
 }
 
@@ skipping 290 matching lines @@
 
   ASSERT(page());
 
   if (m_owner)
     m_owner->setContentFrame(*this);
   else
     page()->setMainFrame(this);
 }
 
 }  // namespace blink