ScriptLoader: Refactor ScriptLoader::ExecuteAsyncTask for its readability.

No change in behavior expected.

- ScriptRunner::GetPendingScriptIfAsync is now renamed to GetPendingScriptOfAsyncScript() and DCHECKs if the script loader is actually async.
- ScriptRunner::IsAsync() is introduced for an existing call-site which called the func just to see if the ScriptLoader is async.

Bug: 838761
Change-Id: I0cbe73c36706e1e4ae053dd3c438d2eb77c62699
Reviewed-on: https://chromium-review.googlesource.com/1036712
Commit-Queue: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Daniel Vogelheim <vogelheim@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#557206}

third_party/blink/renderer/core/script/script_loader.cc

@@ -964,10 +964,9 @@ bool ScriptLoader::IsScriptForEventSupported() const {
          DeprecatedEqualIgnoringCase(event_attribute, "onload()");
 }
 
-PendingScript* ScriptLoader::GetPendingScriptIfScriptIsAsync() {
-  if (pending_script_ && async_exec_type_ == ScriptRunner::kAsync)
-    return pending_script_;
-  return nullptr;
+PendingScript* ScriptLoader::GetPendingScriptIfScriptOfAsyncScript() {
+  DCHECK(IsAsync());
+  return pending_script_;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/script/script_loader.h

@@ -121,16 +121,24 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
   void HandleSourceAttribute(const String& source_url);
   void HandleAsyncAttribute();
 
+  // IsReady only makes sense for scripts controlled by ScriptRunner.
   virtual bool IsReady() const {
     return pending_script_ && pending_script_->IsReady();
   }
 
   void SetFetchDocWrittenScriptDeferIdle();
 
-  // To support script streaming, the ScriptRunner may need to access the
-  // PendingScript. This breaks the intended layering, so please use with
-  // care. (Method is virtual to support testing.)
-  virtual PendingScript* GetPendingScriptIfScriptIsAsync();
+  // IsAsync only makes sense for scripts controlled by ScriptRunner.
+  bool IsAsync() const {
+    DCHECK_NE(async_exec_type_, ScriptRunner::kNone);
+    return async_exec_type_ == ScriptRunner::kAsync;
+  }
+
+  // GetPendingScriptIfScriptOfAsyncScript only makes sense for scripts
+  // controlled by ScriptRunner. To support script streaming, the ScriptRunner
+  // may need to access the PendingScript. This breaks the intended layering, so
+  // please use with care. (Method is virtual to support testing.)
+  virtual PendingScript* GetPendingScriptIfScriptOfAsyncScript();
 
  protected:
   ScriptLoader(ScriptElementBase*,

third_party/blink/renderer/core/script/script_runner.cc

@@ -202,8 +202,7 @@ bool ScriptRunner::ExecuteInOrderTask() {
   if (in_order_scripts_to_execute_soon_.IsEmpty())
     return false;
 
-  DCHECK(!in_order_scripts_to_execute_soon_.front()
-              ->GetPendingScriptIfScriptIsAsync())
+  DCHECK(!in_order_scripts_to_execute_soon_.front()->IsAsync())
       << "In-order scripts queue should not contain any async script.";
 
   in_order_scripts_to_execute_soon_.TakeFirst()->Execute();
@@ -213,18 +212,25 @@ bool ScriptRunner::ExecuteInOrderTask() {
 }
 
 bool ScriptRunner::ExecuteAsyncTask() {
-  for (auto iter = async_scripts_to_execute_soon_.begin();
-       iter != async_scripts_to_execute_soon_.end(); ++iter) {
-    PendingScript* pending_script = (*iter)->GetPendingScriptIfScriptIsAsync();
-    if (!pending_script || !pending_script->IsCurrentlyStreaming()) {
-      ScriptLoader* loader = *iter;
-      async_scripts_to_execute_soon_.erase(iter);
-      loader->Execute();
-      document_->DecrementLoadEventDelayCount();
-      return true;
-    }
+  // Find an async script loader which is not currently streaming.
+  auto it = std::find_if(async_scripts_to_execute_soon_.begin(),
+                         async_scripts_to_execute_soon_.end(),
+                         [](ScriptLoader* loader) {
+                           PendingScript* pending_script =
+                               loader->GetPendingScriptIfScriptOfAsyncScript();
+                           DCHECK(pending_script);
+                           return !pending_script->IsCurrentlyStreaming();
+                         });
+  if (it == async_scripts_to_execute_soon_.end()) {
+    return false;
   }
-  return false;
+
+  // Remove the async script loader from the ready-to-exec set and execute.
+  ScriptLoader* async_script_to_execute = *it;
+  async_scripts_to_execute_soon_.erase(it);
+  async_script_to_execute->Execute();
+  document_->DecrementLoadEventDelayCount();
+  return true;
 }
 
 void ScriptRunner::ExecuteTask() {
@@ -279,7 +285,7 @@ bool ScriptRunner::DoTryStream(ScriptLoader* script_loader) {
                    script_loader) != async_scripts_to_execute_soon_.end());
 
   PendingScript* pending_script =
-      script_loader->GetPendingScriptIfScriptIsAsync();
+      script_loader->GetPendingScriptIfScriptOfAsyncScript();
   if (!pending_script)
     return false;
 

third_party/blink/renderer/core/script/script_runner.h

@@ -84,6 +84,7 @@ class CORE_EXPORT ScriptRunner final
   // Execute the first task in in_order_scripts_to_execute_soon_.
   // Returns true if task was run, and false otherwise.
   bool ExecuteInOrderTask();
+
   // Execute any task in async_scripts_to_execute_soon_.
   // Returns true if task was run, and false otherwise.
   bool ExecuteAsyncTask();

third_party/blink/renderer/core/script/script_runner_test.cc

@@ -104,7 +104,7 @@ class MockScriptLoader final : public ScriptLoader {
 
   void Trace(blink::Visitor*) override;
 
-  PendingScript* GetPendingScriptIfScriptIsAsync() override {
+  PendingScript* GetPendingScriptIfScriptOfAsyncScript() override {
     return mock_pending_script_if_script_is_async_.Get();
   }
   MockPendingScript* GetMockPendingScript() {