Tidy ScriptRunner pending script handling.

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

Index: third_party/WebKit/Source/core/dom/ScriptRunner.cpp
diff --git a/third_party/WebKit/Source/core/dom/ScriptRunner.cpp b/third_party/WebKit/Source/core/dom/ScriptRunner.cpp
index 92df0ed7d28edb9bde698c9d4c7e396a5b325afe..8ac1b6886ae1673a49cf369bebf5217b1790e4b5 100644
--- a/third_party/WebKit/Source/core/dom/ScriptRunner.cpp
+++ b/third_party/WebKit/Source/core/dom/ScriptRunner.cpp
@@ -25,6 +25,7 @@
 
 #include "core/dom/ScriptRunner.h"
 
+#include <algorithm>
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/dom/ScriptLoader.h"
@@ -109,7 +110,7 @@ void ScriptRunner::notifyScriptReady(ScriptLoader* scriptLoader,
   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).
@@ -136,37 +137,35 @@ void ScriptRunner::notifyScriptReady(ScriptLoader* scriptLoader,
 }
 
 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();
 }
@@ -201,9 +200,10 @@ void ScriptRunner::movePendingScript(Document& oldDocument,
 
 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;
   }