Check if an iframe doesn't get detached twice

third_party/WebKit/Source/core/frame/LocalFrame.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 361 matching lines @@
                                       loadType, KURL(), clientRedirectPolicy));
     request.setClientRedirect(clientRedirectPolicy);
     m_loader.load(request, loadType);
   } else {
     DCHECK_EQ(FrameLoadTypeReload, loadType);
     m_navigationScheduler->scheduleReload();
   }
 }
 
 void LocalFrame::detach(FrameDetachType type) {
+  CHECK(!m_detached);
+  m_detached = true;
+
   PluginScriptForbiddenScope forbidPluginDestructorScripting;
   m_loader.stopAllLoaders();

[dcheng, 2016/10/08 04:59:50]

This can run script by causing a frame to be considered load complete: for an
example stack, see
https://bugs.chromium.org/p/chromium/issues/detail?id=561873#c127.

Note: the stack in that bug is past where we dispatch the load event; it's
actually a little higher in the stack in FrameLoader::checkCompleted, which can
call Document::implicitClose().

   // Don't allow any new child frames to load in this frame: attaching a new
   // child frame during or after detaching children results in an attached
   // frame on a detached DOM tree, which is bad.
   SubframeLoadingDisabler disabler(*document());

[dcheng, 2016/10/08 04:59:49]

At this point, we disallow child frames from being attached. Firefox actually
doesn't disallow this, so there is one more "unload child frames" step in
Firefox's Document::shutdown() equivalent.

   m_loader.dispatchUnloadEvent();

[dcheng, 2016/10/08 04:59:49]

This runs the actual unload / pagehide event, which can run script.

   detachChildren();

[dcheng, 2016/10/08 04:59:49]

This recurses into child frames and calls Frame::detach(), which can also
dispatch events.

 
   // All done if detaching the subframes brought about a detach of this frame
   // also.
   if (!client())

[dcheng, 2016/10/08 04:59:49]

Because we just ran script, we must check that we are still attached: if we are
not, then that means one of the previous scripts already reentrantly removed us.
The rest of the work is already done, so we must early return.

     return;
 
   // stopAllLoaders() needs to be called after detachChildren(), because
   // detachChildren() will trigger the unload event handlers of any child
   // frames, and those event handlers might start a new subresource load in this
   // frame.
   m_loader.stopAllLoaders();

[dcheng, 2016/10/08 04:59:50]

I don't honestly remember what this comment is about. I guess it's if a script
adds a <img> or <link rel="stylesheet" ...> or anything else that could start a
fetch.

   m_loader.detach();

[dcheng, 2016/10/08 04:59:49]

This can also dispatch events: this aborts any XHRs that were still in progress.
This will fire the 'abort' event on the XHR if there is one.

   document()->shutdown();

[dcheng, 2016/10/08 04:59:49]

Now we shutdown the Document. This is not supposed to run script, but apparently
it can...

The history behind
https://chromium.googlesource.com/chromium/src/+/d3e0e6655a412956248d217baa39...
is complicated.

Originally, when I cleaned up this code, executing script in a
ScriptForbiddenScope was a non-fatal error. So I added a ScriptForbiddenScope to
prevent script from running in the middle of detaching a DOM node (Document).

Then we made executing script in ScriptForbiddenScope a fatal error. So esprehn@
added HTMLFrameOwnerElement::UpdateSuspendScope to prevent plugins from running
script inside ScriptForbiddenScope.

However, since plugins could now run script in Document::detach, they were able
to queue navigations that would violate invariants during detach. So we added
FrameNavigationDisabler to prevent that...

   // This is the earliest that scripting can be disabled:
   // - FrameLoader::detach() can fire XHR abort events
   // - Document::shutdown()'s deferred widget updates can run script.
   ScriptForbiddenScope forbidScript;

[dcheng, 2016/10/08 04:59:49]

Now no more script is allowed, and everything is a lot simpler =)

   m_loader.clear();
   if (!client())

[dcheng, 2016/10/08 04:59:49]

However, I'm not sure why this check is after m_loader.clear() instead of
before. It should probably be before m_loader.clear()...

     return;
 
   client()->willBeDetached();
   // Notify ScriptController that the frame is closing, since its cleanup ends
   // up calling back to FrameLoaderClient via WindowProxy.
   script().clearForClose();
   setView(nullptr);
 
   m_host->eventHandlerRegistry().didRemoveAllEventHandlers(*localDOMWindow());
 
@@ skipping 429 matching lines @@
       m_editor(Editor::create(*this)),
       m_spellChecker(SpellChecker::create(*this)),
       m_selection(FrameSelection::create(this)),
       m_eventHandler(new EventHandler(this)),
       m_console(FrameConsole::create(*this)),
       m_inputMethodController(InputMethodController::create(*this)),
       m_navigationDisableCount(0),
       m_pageZoomFactor(parentPageZoomFactor(this)),
       m_textZoomFactor(parentTextZoomFactor(this)),
       m_inViewSourceMode(false),
-      m_interfaceProvider(interfaceProvider) {
+      m_interfaceProvider(interfaceProvider),
+      m_detached(false) {
   if (isLocalRoot())
     m_instrumentingAgents = new InstrumentingAgents();
   else
     m_instrumentingAgents = localFrameRoot()->m_instrumentingAgents;
 }
 
 WebFrameScheduler* LocalFrame::frameScheduler() {
   return m_frameScheduler.get();
 }
 
@@ skipping 29 matching lines @@
   if (m_frame && m_frame->client() && m_frame->client()->frameBlameContext())
     m_frame->client()->frameBlameContext()->Enter();
 }
 
 ScopedFrameBlamer::~ScopedFrameBlamer() {
   if (m_frame && m_frame->client() && m_frame->client()->frameBlameContext())
     m_frame->client()->frameBlameContext()->Leave();
 }
 
 }  // namespace blink