Tidy ScriptRunner pending script handling.

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
  *    documentation and/or other materials provided with the distribution.
  *
  * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
  * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
  * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
  * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
  * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
  * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
  * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #include "core/dom/ScriptRunner.h"
 
+#include <algorithm>
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/dom/ScriptLoader.h"
 #include "core/dom/TaskRunnerHelper.h"
 #include "platform/heap/Handle.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebScheduler.h"
 #include "public/platform/WebThread.h"
 
 namespace blink {
@@ skipping 64 matching lines @@
     m_inOrderScriptsToExecuteSoon.append(m_pendingInOrderScripts.takeFirst());
     postTask(BLINK_FROM_HERE);
   }
 }
 
 void ScriptRunner::notifyScriptReady(ScriptLoader* scriptLoader,
                                      AsyncExecutionType executionType) {
   SECURITY_CHECK(scriptLoader);
   switch (executionType) {
     case Async:
-      // RELEASE_ASSERT makes us crash in a controlled way in error cases
+      // SECURITY_CHECK() 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).
       SECURITY_CHECK(m_pendingAsyncScripts.contains(scriptLoader));
 
       m_pendingAsyncScripts.erase(scriptLoader);
       m_asyncScriptsToExecuteSoon.append(scriptLoader);
 
       postTask(BLINK_FROM_HERE);
 
       break;
 
     case InOrder:
       SECURITY_CHECK(m_numberOfInOrderScriptsWithPendingNotification > 0);
       m_numberOfInOrderScriptsWithPendingNotification--;
 
       scheduleReadyInOrderScripts();
 
       break;
     case None:
       NOTREACHED();
       break;
   }
 }
 
 bool ScriptRunner::removePendingInOrderScript(ScriptLoader* scriptLoader) {
-  for (auto it = m_pendingInOrderScripts.begin();
-       it != m_pendingInOrderScripts.end(); ++it) {
-    if (*it == scriptLoader) {
-      m_pendingInOrderScripts.remove(it);
-      SECURITY_CHECK(m_numberOfInOrderScriptsWithPendingNotification > 0);
-      m_numberOfInOrderScriptsWithPendingNotification--;
-      return true;
-    }
-  }
-  return false;
+  auto it = std::find_if(

[hiroshige, 2017/02/23 17:07:19]

Lack of (Heap)Deque::find() looks causing similar code, e.g.
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/f...

It would be nice if we can provide a more convenient way, e.g. std::find()
rather than using anonymous comparator, or defining Deque::find(), and then
replace similar code patterns (I could find only one above though).

[sof, 2017/02/23 19:15:30]

Simplified to std::find(), Member<T> does have an implicit conversion to T*, as
wanted.

+      m_pendingInOrderScripts.begin(), m_pendingInOrderScripts.end(),
+      [=](ScriptLoader* loader) { return loader == scriptLoader; });
+  if (it == m_pendingInOrderScripts.end())
+    return false;
+  m_pendingInOrderScripts.remove(it);
+  SECURITY_CHECK(m_numberOfInOrderScriptsWithPendingNotification > 0);
+  m_numberOfInOrderScriptsWithPendingNotification--;
+  return true;
 }
 
 void ScriptRunner::notifyScriptLoadError(ScriptLoader* scriptLoader,
                                          AsyncExecutionType executionType) {
   switch (executionType) {
     case Async: {
-      // SECURITY_CHECK 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).
+      // See notifyScriptReady() comment.
       SECURITY_CHECK(m_pendingAsyncScripts.contains(scriptLoader));
       m_pendingAsyncScripts.erase(scriptLoader);
       break;
     }
-    case InOrder:
+    case InOrder: {
       SECURITY_CHECK(removePendingInOrderScript(scriptLoader));
       scheduleReadyInOrderScripts();
       break;
-    case None:
+    }
+    case None: {
       NOTREACHED();
       break;
+    }
   }
   m_document->decrementLoadEventDelayCount();
 }
 
 void ScriptRunner::movePendingScript(Document& oldDocument,
                                      Document& newDocument,
                                      ScriptLoader* scriptLoader) {
   Document* newContextDocument = newDocument.contextDocument();
   if (!newContextDocument) {
     // Document's contextDocument() method will return no Document if the
@@ skipping 14 matching lines @@
     DCHECK(!oldDocument.frame());
     oldContextDocument = &oldDocument;
   }
   if (oldContextDocument != newContextDocument)
     oldContextDocument->scriptRunner()->movePendingScript(
         newContextDocument->scriptRunner(), scriptLoader);
 }
 
 void ScriptRunner::movePendingScript(ScriptRunner* newRunner,
                                      ScriptLoader* scriptLoader) {
-  if (m_pendingAsyncScripts.contains(scriptLoader)) {
+  auto it = m_pendingAsyncScripts.find(scriptLoader);
+  if (it != m_pendingAsyncScripts.end()) {
     newRunner->queueScriptForExecution(scriptLoader, Async);
-    m_pendingAsyncScripts.erase(scriptLoader);
+    m_pendingAsyncScripts.erase(it);
     m_document->decrementLoadEventDelayCount();
     return;
   }
   if (removePendingInOrderScript(scriptLoader)) {
     newRunner->queueScriptForExecution(scriptLoader, InOrder);
     m_document->decrementLoadEventDelayCount();
   }
 }
 
 // Returns true if task was run, and false otherwise.
@@ skipping 25 matching lines @@
 
 DEFINE_TRACE(ScriptRunner) {
   visitor->trace(m_document);
   visitor->trace(m_pendingInOrderScripts);
   visitor->trace(m_pendingAsyncScripts);
   visitor->trace(m_asyncScriptsToExecuteSoon);
   visitor->trace(m_inOrderScriptsToExecuteSoon);
 }
 
 }  // namespace blink