Check that PendingScript::m_streamer is always null when resource() is null

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 40 matching lines @@
                              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),
       m_isForTesting(isForTesting) {
-  CHECK(m_isForTesting || m_element);
+  checkState();
   setResource(resource);
   MemoryCoordinator::instance().registerClient(this);
 }
 
 PendingScript::~PendingScript() {}
 
+NOINLINE void PendingScript::checkState() const {
+  // TODO(hiroshige): Turn these CHECK()s into DCHECK() before going to beta.
+  CHECK(m_isForTesting || m_element);
+  CHECK(resource() || !m_streamer);
+  CHECK(!m_streamer || m_streamer->resource() == resource());
+}
+
 void PendingScript::dispose() {
+  checkState();

[kouhei (in TOK), 2017/02/23 01:54:17]

Doesn't have to be this CL, but calling dispose() on dispose()d PendingScript
seems wrong...

[hiroshige, 2017/02/23 01:58:51]

dispose() is the prefinalizer of PendingScript, so it doesn't necessarily wrong
for me that the prefinalizer is called after someone explicitly calls dispose()
before that.

[sof, 2017/02/23 07:26:23]

Yes, promptly disposing of this object is worthwhile to release resources a
little bit earlier. The lifetime end we know most of the time for PendingScript,
but not always, hence rely on prefinalizers for the rest.

We don't support unregistering prefinalizers, and I don't think we should/need
to here.

+
   stopWatchingForLoad();
   DCHECK(!m_client);
   DCHECK(!m_watchingForLoad);
 
   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) {
+  checkState();
+
   DCHECK(!m_watchingForLoad);
   // addClient() will call streamingFinished() if the load is complete. Callers
   // who do not expect to be re-entered from this call should not call
   // watchForLoad for a PendingScript which isReady. We also need to set
   // m_watchingForLoad early, since addClient() can result in calling
   // notifyFinished and further stopWatchingForLoad().
   m_watchingForLoad = true;
   m_client = client;
   if (isReady())
     m_client->pendingScriptFinished(this);
 }
 
 void PendingScript::stopWatchingForLoad() {
+  checkState();
+
   if (!m_watchingForLoad)
     return;
   DCHECK(resource());
   m_client = nullptr;
   m_watchingForLoad = false;
 }
 
 Element* PendingScript::element() const {
   // As mentioned in the comment at |m_element| declaration, |m_element|
   // must points to the corresponding ScriptLoader's element.
   CHECK(m_element);
   return m_element.get();
 }
 
 void PendingScript::streamingFinished() {
+  checkState();
   DCHECK(resource());
   if (m_client)
     m_client->pendingScriptFinished(this);
 }
 
 void PendingScript::markParserBlockingLoadStartTime() {
   DCHECK_EQ(m_parserBlockingLoadStartTime, 0.0);
   m_parserBlockingLoadStartTime = monotonicallyIncreasingTime();
 }
 
@@ skipping 56 matching lines @@
   //
   // 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);
+  checkState();
   if (m_element)
     m_integrityFailure = !checkScriptResourceIntegrity(resource, m_element);
 
   // If script streaming is in use, the client will be notified in
   // streamingFinished.
   if (m_streamer)
     m_streamer->notifyFinished(resource);
   else if (m_client)
     m_client->pendingScriptFinished(this);
 }
 
 void PendingScript::notifyAppendData(ScriptResource* resource) {
   if (m_streamer)
     m_streamer->notifyAppendData(resource);
 }
 
 DEFINE_TRACE(PendingScript) {
   visitor->trace(m_element);
   visitor->trace(m_streamer);
   visitor->trace(m_client);
   ResourceOwner<ScriptResource>::trace(visitor);
   MemoryCoordinatorClient::trace(visitor);
 }
 
 ScriptSourceCode PendingScript::getSource(const KURL& documentURL,
                                           bool& errorOccurred) const {
+  checkState();
+
   if (resource()) {
     errorOccurred = resource()->errorOccurred() || m_integrityFailure;
     DCHECK(resource()->isLoaded());
     if (m_streamer && !m_streamer->streamingSuppressed())
       return ScriptSourceCode(m_streamer, resource());
     return ScriptSourceCode(resource());
   }
+
   errorOccurred = false;
   return ScriptSourceCode(m_element->textContent(), documentURL,
                           startingPosition());
 }
 
 void PendingScript::setStreamer(ScriptStreamer* streamer) {
   DCHECK(!m_streamer);
   DCHECK(!m_watchingForLoad);
   m_streamer = streamer;
+  checkState();
 }
 
 bool PendingScript::isReady() const {
-  if (resource() && !resource()->isLoaded())
-    return false;
-  if (m_streamer && !m_streamer->isFinished())
-    return false;
+  checkState();
+  if (resource()) {
+    return resource()->isLoaded() && (!m_streamer || m_streamer->isFinished());
+  }
+
   return true;
 }
 
 bool PendingScript::errorOccurred() const {
+  checkState();
   if (resource())
     return resource()->errorOccurred();
-  if (m_streamer && m_streamer->resource())
-    return m_streamer->resource()->errorOccurred();
+
   return false;
 }
 
 void PendingScript::onPurgeMemory() {
+  checkState();
   if (!m_streamer)
     return;
   m_streamer->cancel();
   m_streamer = nullptr;
 }
 
 }  // namespace blink