Use IsInDocumentWrite() at the time of PrepareScript() instead of element creation

ScriptLoader and related classes use two document.write()-related flags:
- Document::IsInDocumentWrite() at the time of PrepareScript():
  directly checked within PrepareScript() and used for multiple purposes.
- Document::IsInDocumentWrite() at the time of <script> element creation:
  plumbed from HTML parser to ScriptLoader::created_during_document_write_
  and used for DocumentParserTiming.

This CL merges the latter into the former:
- Moves ScriptLoader::created_during_document_write_ to
  PendingScript::created_during_document_write_ which is set to IsInDocumentWrite()
  at the time of PendingScript creation, which is within PrepareScript(), and
- Removes the plumbing of the flag from the HTML parser to ScriptLoader.

Thus this CL simplifies the code by reducing plumbing through ScriptLoader constructor.

Bug: 842349
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I2b17dd098bb134a59b9c1ba2937c74bfa46b4c0f
Reviewed-on: https://chromium-review.googlesource.com/1053163
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#560739}

third_party/blink/renderer/core/dom/create_element_flags.h

@@ -16,9 +16,6 @@ class CreateElementFlags {
   bool IsCustomElementsV1() const { return custom_elements_v1_; }
   bool IsCustomElementsV0() const { return custom_elements_v0_; }
   bool WasAlreadyStarted() const { return already_started_; }
-  bool IsCreatedDuringDocumentWrite() const {
-    return created_during_document_write_;
-  }
 
   // https://html.spec.whatwg.org/#create-an-element-for-the-token
   static CreateElementFlags ByParser() {
@@ -52,8 +49,7 @@ class CreateElementFlags {
         async_custom_elements_(false),
         custom_elements_v1_(true),
         custom_elements_v0_(true),
-        already_started_(false),
-        created_during_document_write_(false) {}
+        already_started_(false) {}
 
   CreateElementFlags& SetCreatedByParser(bool flag) {
     created_by_parser_ = flag;
@@ -66,12 +62,6 @@ class CreateElementFlags {
     return *this;
   }
 
-  // For <script>.
-  CreateElementFlags& SetCreatedDuringDocumentWrite(bool flag) {
-    created_during_document_write_ = flag;
-    return *this;
-  }
-
  private:
   CreateElementFlags& SetAsyncCustomElements() {
     async_custom_elements_ = true;
@@ -96,7 +86,6 @@ class CreateElementFlags {
   bool custom_elements_v0_ : 1;
 
   bool already_started_ : 1;
-  bool created_during_document_write_ : 1;
 };
 
 #endif  // THIRD_PARTY_BLINK_RENDERER_CORE_DOM_CREATE_ELEMENT_FLAGS_H_

third_party/blink/renderer/core/html/html_script_element.cc

@@ -44,8 +44,7 @@ inline HTMLScriptElement::HTMLScriptElement(Document& document,
                                             const CreateElementFlags flags)
     : HTMLElement(scriptTag, document),
       loader_(InitializeScriptLoader(flags.IsCreatedByParser(),
-                                     flags.WasAlreadyStarted(),
-                                     flags.IsCreatedDuringDocumentWrite())) {}
+                                     flags.WasAlreadyStarted())) {}
 
 HTMLScriptElement* HTMLScriptElement::Create(Document& document,
                                              const CreateElementFlags flags) {

third_party/blink/renderer/core/html/parser/html_construction_site.cc

@@ -732,14 +732,7 @@ void HTMLConstructionSite::InsertScriptElement(AtomicHTMLToken* token) {
       // elements since scripts can never see those flags or effects thereof.
       .SetCreatedByParser(parser_content_policy_ !=
                           kAllowScriptingContentAndDoNotMarkAlreadyStarted)
-      .SetAlreadyStarted(is_parsing_fragment_ && flags.IsCreatedByParser())
-      // TODO(csharrison): This logic only works if the tokenizer/parser was not
-      // blocked waiting for scripts when the element was inserted. This usually
-      // fails for instance, on second document.write if a script writes twice
-      // in a row. To fix this, the parser might have to keep track of raw
-      // string position.
-      .SetCreatedDuringDocumentWrite(
-          OwnerDocumentForCurrentNode().IsInDocumentWrite());
+      .SetAlreadyStarted(is_parsing_fragment_ && flags.IsCreatedByParser());
   HTMLScriptElement* element = nullptr;
   if (const auto* is_attribute = token->GetAttributeItem(HTMLNames::isAttr)) {
     element = ToHTMLScriptElement(OwnerDocumentForCurrentNode().CreateElement(

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

@@ -47,6 +47,8 @@ WebScopedVirtualTimePauser CreateWebScopedVirtualTimePauser(
 }
 }  // namespace
 
+// See the comment about |is_in_document_write| in ScriptLoader::PrepareScript()
+// about IsInDocumentWrite() use here.
 PendingScript::PendingScript(ScriptElementBase* element,
                              const TextPosition& starting_position)
     : element_(element),
@@ -54,7 +56,9 @@ PendingScript::PendingScript(ScriptElementBase* element,
       parser_blocking_load_start_time_(0),
       virtual_time_pauser_(CreateWebScopedVirtualTimePauser(element)),
       client_(nullptr),
-      original_context_document_(element->GetDocument().ContextDocument()) {}
+      original_context_document_(element->GetDocument().ContextDocument()),
+      created_during_document_write_(
+          element->GetDocument().IsInDocumentWrite()) {}
 
 PendingScript::~PendingScript() {}
 

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

@@ -132,6 +132,10 @@ class CORE_EXPORT PendingScript
     return original_context_document_;
   }
 
+  bool WasCreatedDuringDocumentWrite() {
+    return created_during_document_write_;
+  }
+
  protected:
   PendingScript(ScriptElementBase*, const TextPosition& starting_position);
 
@@ -161,6 +165,8 @@ class CORE_EXPORT PendingScript
   // documents and thus doesn't retain a strong reference.
   WeakMember<Document> original_context_document_;
 
+  const bool created_during_document_write_;
+
   DISALLOW_COPY_AND_ASSIGN(PendingScript);
 };
 

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

@@ -17,12 +17,9 @@ ScriptElementBase* ScriptElementBase::FromElementIfPossible(Element* element) {
   return nullptr;
 }
 
-ScriptLoader* ScriptElementBase::InitializeScriptLoader(
-    bool parser_inserted,
-    bool already_started,
-    bool created_during_document_write) {
-  return ScriptLoader::Create(this, parser_inserted, already_started,
-                              created_during_document_write);
+ScriptLoader* ScriptElementBase::InitializeScriptLoader(bool parser_inserted,
+                                                        bool already_started) {
+  return ScriptLoader::Create(this, parser_inserted, already_started);
 }
 
 }  // namespace blink

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

@@ -70,8 +70,7 @@ class CORE_EXPORT ScriptElementBase : public GarbageCollectedMixin {
 
  protected:
   ScriptLoader* InitializeScriptLoader(bool parser_inserted,
-                                       bool already_started,
-                                       bool created_during_document_write);
+                                       bool already_started);
 
   friend class ScriptLoader;
   virtual void DispatchLoadEvent() = 0;

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

@@ -64,12 +64,10 @@ namespace blink {
 
 ScriptLoader::ScriptLoader(ScriptElementBase* element,
                            bool parser_inserted,
-                           bool already_started,
-                           bool created_during_document_write)
+                           bool already_started)
     : element_(element),
       will_be_parser_executed_(false),
-      will_execute_when_document_finished_parsing_(false),
-      created_during_document_write_(created_during_document_write) {
+      will_execute_when_document_finished_parsing_(false) {
   // <spec
   // href="https://html.spec.whatwg.org/multipage/scripting.html#already-started">
   // ... The cloning steps for script elements must set the "already started"
@@ -361,10 +359,18 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
 
   DCHECK(!prepared_pending_script_);
 
+  // TODO(csharrison): This logic only works if the tokenizer/parser was not
+  // blocked waiting for scripts when the element was inserted. This usually
+  // fails for instance, on second document.write if a script writes twice
+  // in a row. To fix this, the parser might have to keep track of raw
+  // string position.
+  //
+  // Also PendingScript's contructor has the same code.
+  const bool is_in_document_write = element_document.IsInDocumentWrite();
+
   // Reset line numbering for nested writes.
-  TextPosition position = element_document.IsInDocumentWrite()
-                              ? TextPosition()
-                              : script_start_position;
+  TextPosition position =
+      is_in_document_write ? TextPosition() : script_start_position;
 
   // <spec step="21">Let options be a script fetch options whose cryptographic
   // nonce is cryptographic nonce, integrity metadata is integrity metadata,
@@ -491,7 +497,7 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
         if (!parser_inserted_) {
           script_location_type =
               ScriptSourceLocationType::kInlineInsideGeneratedElement;
-        } else if (element_->GetDocument().IsInDocumentWrite()) {
+        } else if (is_in_document_write) {
           script_location_type =
               ScriptSourceLocationType::kInlineInsideDocumentWrite;
         }
@@ -680,7 +686,7 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
   // Note: this block is also duplicated in
   // HTMLParserScriptRunner::processScriptElementInternal().
   // TODO(hiroshige): Merge the duplicated code.
-  KURL script_url = (!element_document.IsInDocumentWrite() && parser_inserted_)
+  KURL script_url = (!is_in_document_write && parser_inserted_)
                         ? element_document.Url()
                         : KURL();
   ExecuteScriptBlock(TakePendingScript(ScriptSchedulingType::kImmediate),
@@ -809,6 +815,8 @@ void ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script,
 
   const bool was_canceled = pending_script->WasCanceled();
   const bool is_external = pending_script->IsExternal();
+  const bool created_during_document_write =
+      pending_script->WasCreatedDuringDocumentWrite();
   const double parser_blocking_load_start_time =
       pending_script->ParserBlockingLoadStartTime();
   pending_script->Dispose();
@@ -824,7 +832,7 @@ void ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script,
     DocumentParserTiming::From(element_->GetDocument())
         .RecordParserBlockedOnScriptLoadDuration(
             CurrentTimeTicksInSeconds() - parser_blocking_load_start_time,
-            WasCreatedDuringDocumentWrite());
+            created_during_document_write);
   }
 
   if (was_canceled)
@@ -907,7 +915,7 @@ void ScriptLoader::ExecuteScriptBlock(PendingScript* pending_script,
     DocumentParserTiming::From(element_->GetDocument())
         .RecordParserBlockedOnScriptExecutionDuration(
             CurrentTimeTicksInSeconds() - script_exec_start_time,
-            WasCreatedDuringDocumentWrite());
+            created_during_document_write);
   }
 
   // <spec step="8">If the script is from an external file, then fire an event

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

@@ -55,10 +55,8 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
  public:
   static ScriptLoader* Create(ScriptElementBase* element,
                               bool created_by_parser,
-                              bool is_evaluated,
-                              bool created_during_document_write) {
-    return new ScriptLoader(element, created_by_parser, is_evaluated,
-                            created_during_document_write);
+                              bool is_evaluated) {
+    return new ScriptLoader(element, created_by_parser, is_evaluated);
   }
 
   ~ScriptLoader() override;
@@ -124,10 +122,7 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
   virtual PendingScript* GetPendingScriptIfControlledByScriptRunner();
 
  protected:
-  ScriptLoader(ScriptElementBase*,
-               bool created_by_parser,
-               bool is_evaluated,
-               bool created_during_document_write);
+  ScriptLoader(ScriptElementBase*, bool created_by_parser, bool is_evaluated);
 
  private:
   bool IgnoresLoadRequest() const;
@@ -156,10 +151,6 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
   // PendingScriptClient
   void PendingScriptFinished(PendingScript*) override;
 
-  bool WasCreatedDuringDocumentWrite() {
-    return created_during_document_write_;
-  }
-
   Member<ScriptElementBase> element_;
 
   // https://html.spec.whatwg.org/multipage/scripting.html#script-processing-model
@@ -200,8 +191,6 @@ class CORE_EXPORT ScriptLoader : public GarbageCollectedFinalized<ScriptLoader>,
 
   bool will_execute_when_document_finished_parsing_;
 
-  const bool created_during_document_write_;
-
   // A PendingScript is first created in PrepareScript() and stored in
   // |prepared_pending_script_|.
   // Later, TakePendingScript() is called, and its caller holds a reference

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

@@ -114,7 +114,7 @@ class MockScriptLoader final : public ScriptLoader {
 
  private:
   explicit MockScriptLoader(ScriptElementBase* element)
-      : ScriptLoader(element, false, false, false) {}
+      : ScriptLoader(element, false, false) {}
 
   static MockScriptLoader* Create(Document* document,
                                   ScriptSchedulingType scheduling_type) {

third_party/blink/renderer/core/svg/svg_script_element.cc

@@ -37,8 +37,7 @@ inline SVGScriptElement::SVGScriptElement(Document& document,
     : SVGElement(SVGNames::scriptTag, document),
       SVGURIReference(this),
       loader_(InitializeScriptLoader(flags.IsCreatedByParser(),
-                                     flags.WasAlreadyStarted(),
-                                     false)) {}
+                                     flags.WasAlreadyStarted())) {}
 
 SVGScriptElement* SVGScriptElement::Create(Document& document,
                                            const CreateElementFlags flags) {