Let notifyScriptLoadError() handle already detached ScriptLoaders.

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

 /*
  * Copyright (C) 2010 Google, Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 186 matching lines @@
 }
 
 void ScriptRunner::notifyScriptLoadError(ScriptLoader* scriptLoader, ExecutionType executionType)
 {
     switch (executionType) {
     case ASYNC_EXECUTION: {
         // RELEASE_ASSERT makes us crash in a controlled way in error cases
         // where the ScriptLoader is associated with the wrong ScriptRunner
         // (otherwise we'd cause a use-after-free in ~ScriptRunner when it tries
         // to detach).
-        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(m_pendingAsyncScripts.contains(scriptLoader));
+        bool foundLoader = m_pendingAsyncScripts.contains(scriptLoader);
+        foundLoader = foundLoader || (scriptLoader->isDetached() && m_pendingAsyncScripts.isEmpty());

[haraken, 2016/01/27 08:46:46]

I understand the part of 'scriptLoader->isDetached()', but why do you need the
part of '&& m_pendingAsyncScripts.isEmpty()'?

[sof, 2016/01/27 09:10:11]

It is an indication of the ScriptRunner being in a disposed state, without
tracking that with a separate flag. i.e., I want to know the reason why the
ScriptLoader is detached (by the dispose() having happened), otherwise I don't
understand the reasons for how that can come to be.

[haraken, 2016/01/27 09:13:07]

Thanks, maybe worth adding a comment.

+        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(foundLoader);

[haraken, 2016/01/27 08:46:46]

Nit: I'd write RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(foundLoader || ...)
here.

[sof, 2016/01/27 09:10:11]

Sorry about the slight confusion - the previous line will be !ENABLE(OILPAN),
see ps#2, which is why it looks a bit odd.

         m_pendingAsyncScripts.remove(scriptLoader);
         break;
     }
     case IN_ORDER_EXECUTION:
-        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(removePendingInOrderScript(scriptLoader));
+        bool foundLoader = removePendingInOrderScript(scriptLoader);
+        foundLoader = foundLoader || (scriptLoader->isDetached() && m_pendingInOrderScripts.isEmpty());
+        RELEASE_ASSERT_WITH_SECURITY_IMPLICATION(foundLoader);
         break;
     }
     scriptLoader->detach();
     m_document->decrementLoadEventDelayCount();
 }
 
 void ScriptRunner::movePendingScript(Document& oldDocument, Document& newDocument, ScriptLoader* scriptLoader)
 {
     RefPtrWillBeRawPtr<Document> newContextDocument = newDocument.contextDocument().get();
     if (!newContextDocument) {
@@ skipping 66 matching lines @@
 #if ENABLE(OILPAN)
     visitor->trace(m_document);
     visitor->trace(m_pendingInOrderScripts);
     visitor->trace(m_pendingAsyncScripts);
     visitor->trace(m_asyncScriptsToExecuteSoon);
     visitor->trace(m_inOrderScriptsToExecuteSoon);
 #endif
 }
 
 } // namespace blink