Do not re-initialize PendingScript in HTMLParserScriptRunner

third_party/WebKit/Source/core/dom/PendingScript.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 17 matching lines @@
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "core/dom/Element.h"
 #include "core/frame/SubresourceIntegrity.h"
 #include "platform/SharedBuffer.h"
 #include "wtf/CurrentTime.h"
 
 namespace blink {
 
 PendingScript* PendingScript::create(Element* element,
                                      ScriptResource* resource) {
-  return new PendingScript(element, resource);
+  return new PendingScript(element, resource, TextPosition());
 }
 
-PendingScript::PendingScript(Element* element, ScriptResource* resource)
+PendingScript* PendingScript::create(Element* element,
+                                     const TextPosition& startingPosition) {
+  return new PendingScript(element, nullptr, startingPosition);
+}
+
+PendingScript* PendingScript::createForTesting(ScriptResource* resource) {
+  return new PendingScript(nullptr, resource, TextPosition(), true);
+}
+
+PendingScript::PendingScript(Element* element,
+                             ScriptResource* resource,
+                             const TextPosition& startingPosition,
+                             bool isForTesting)
     : m_watchingForLoad(false),
       m_element(element),
+      m_startingPosition(startingPosition),
       m_integrityFailure(false),
       m_parserBlockingLoadStartTime(0),
-      m_client(nullptr) {
-  setScriptResource(resource);
+      m_client(nullptr),
+      m_isForTesting(isForTesting) {
+  CHECK(m_isForTesting || m_element);
+  setResource(resource);
   MemoryCoordinator::instance().registerClient(this);
 }
 
 PendingScript::~PendingScript() {}
 
 void PendingScript::dispose() {
   stopWatchingForLoad();
   DCHECK(!m_client);
   DCHECK(!m_watchingForLoad);
 
-  setScriptResource(nullptr);
+  setResource(nullptr);
   m_startingPosition = TextPosition::belowRangePosition();
   m_integrityFailure = false;
   m_parserBlockingLoadStartTime = 0;
   if (m_streamer)
     m_streamer->cancel();
   m_streamer = nullptr;
   m_element = nullptr;
 }
 
 void PendingScript::watchForLoad(PendingScriptClient* client) {
@@ skipping 10 matching lines @@
 }
 
 void PendingScript::stopWatchingForLoad() {
   if (!m_watchingForLoad)
     return;
   DCHECK(resource());
   m_client = nullptr;
   m_watchingForLoad = false;
 }
 
+Element* PendingScript::element() const {
+  CHECK(m_element);

[sof, 2017/02/16 08:15:15]

Could you xref the element() declaration's comment, to clarify why the CHECK()
is used?

[hiroshige, 2017/02/17 23:15:15]

Done.

+  return m_element.get();
+}
+
 void PendingScript::streamingFinished() {
   DCHECK(resource());
   if (m_client)
     m_client->pendingScriptFinished(this);
 }
 
-void PendingScript::setElement(Element* element) {
-  m_element = element;
-}
-
-void PendingScript::setScriptResource(ScriptResource* resource) {
-  setResource(resource);
-}
-
 void PendingScript::markParserBlockingLoadStartTime() {
   DCHECK_EQ(m_parserBlockingLoadStartTime, 0.0);
   m_parserBlockingLoadStartTime = monotonicallyIncreasingTime();
 }
 
 void PendingScript::notifyFinished(Resource* resource) {
   // The following SRI checks need to be here because, unfortunately, fetches
   // are not done purely according to the Fetch spec. In particular,
   // different requests for the same resource do not have different
   // responses; the memory cache can (and will) return the exact same
   // Resource object.
   //
   // For different requests, the same Resource object will be returned and
   // will not be associated with the particular request.  Therefore, when the
   // body of the response comes in, there's no way to validate the integrity
   // of the Resource object against a particular request (since there may be
   // several pending requests all tied to the identical object, and the
   // actual requests are not stored).
   //
   // In order to simulate the correct behavior, Blink explicitly does the SRI
   // checks here, when a PendingScript tied to a particular request is
   // finished (and in the case of a StyleSheet, at the point of execution),
   // while having proper Fetch checks in the fetch module for use in the
   // fetch JavaScript API. In a future world where the ResourceFetcher uses
   // the Fetch algorithm, this should be fixed by having separate Response
   // objects (perhaps attached to identical Resource objects) per request.
   //
   // See https://crbug.com/500701 for more information.
+  CHECK(m_isForTesting || m_element);
   if (m_element) {

[sof, 2017/02/16 08:15:15]

Not part of this CL at all, but moving this SRI-specific special case out into a
separate method, would read better.

[hiroshige, 2017/02/17 23:15:15]

Created https://codereview.chromium.org/2698613007/.

     DCHECK_EQ(resource->getType(), Resource::Script);
     ScriptResource* scriptResource = toScriptResource(resource);
     String integrityAttr =
         m_element->fastGetAttribute(HTMLNames::integrityAttr);
 
     // It is possible to get back a script resource with integrity metadata
     // for a request with an empty integrity attribute. In that case, the
     // integrity check should be skipped, so this check ensures that the
     // integrity attribute isn't empty in addition to checking if the
     // resource has empty integrity metadata.
@@ skipping 77 matching lines @@
 }
 
 void PendingScript::onPurgeMemory() {
   if (!m_streamer)
     return;
   m_streamer->cancel();
   m_streamer = nullptr;
 }
 
 }  // namespace blink