Reland of Split PendingScript into PendingScript and ClassicPendingScript

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

Index: third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
diff --git a/third_party/WebKit/Source/core/dom/PendingScript.cpp b/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
similarity index 53%
copy from third_party/WebKit/Source/core/dom/PendingScript.cpp
copy to third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
index 602f40737a1429d8cabad88b6b4e0f6984517e3a..589159b27e7245fa70c5be64a17c526470c2eeea 100644
--- a/third_party/WebKit/Source/core/dom/PendingScript.cpp
+++ b/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
@@ -1,141 +1,77 @@
-/*
- * 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
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "core/dom/PendingScript.h"
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/dom/ClassicPendingScript.h"
 
 #include "bindings/core/v8/ScriptSourceCode.h"
 #include "bindings/core/v8/ScriptState.h"
+#include "bindings/core/v8/ScriptStreamer.h"
 #include "bindings/core/v8/V8BindingForCore.h"
-#include "core/dom/ClassicScript.h"
 #include "core/dom/Document.h"
-#include "core/dom/ScriptElementBase.h"
 #include "core/dom/TaskRunnerHelper.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/SubresourceIntegrity.h"
-#include "platform/SharedBuffer.h"
-#include "platform/wtf/CurrentTime.h"
+#include "platform/loader/fetch/MemoryCache.h"
 
 namespace blink {
 
-PendingScript* PendingScript::Create(ScriptElementBase* element,
-                                     ScriptResource* resource) {
-  return new PendingScript(element, resource, TextPosition());
+ClassicPendingScript* ClassicPendingScript::Create(ScriptElementBase* element,
+                                                   ScriptResource* resource) {
+  return new ClassicPendingScript(element, resource, TextPosition());
 }
 
-PendingScript* PendingScript::Create(ScriptElementBase* element,
-                                     const TextPosition& starting_position) {
-  return new PendingScript(element, nullptr, starting_position);
+ClassicPendingScript* ClassicPendingScript::Create(
+    ScriptElementBase* element,
+    const TextPosition& starting_position) {
+  return new ClassicPendingScript(element, nullptr, starting_position);
 }
 
-PendingScript* PendingScript::CreateForTesting(ScriptResource* resource) {
-  return new PendingScript(nullptr, resource, TextPosition(), true);
+ClassicPendingScript* ClassicPendingScript::CreateForTesting(
+    ScriptResource* resource) {
+  return new ClassicPendingScript(nullptr, resource, TextPosition(), true);
 }
 
-PendingScript::PendingScript(ScriptElementBase* element,
-                             ScriptResource* resource,
-                             const TextPosition& starting_position,
-                             bool is_for_testing)
-    : watching_for_load_(false),
-      element_(element),
-      starting_position_(starting_position),
+ClassicPendingScript::ClassicPendingScript(
+    ScriptElementBase* element,
+    ScriptResource* resource,
+    const TextPosition& starting_position,
+    bool is_for_testing)
+    : PendingScript(element, starting_position),
       integrity_failure_(false),
-      parser_blocking_load_start_time_(0),
-      client_(nullptr),
       is_for_testing_(is_for_testing) {
   CheckState();
   SetResource(resource);
   MemoryCoordinator::Instance().RegisterClient(this);
 }
 
-PendingScript::~PendingScript() {}
+ClassicPendingScript::~ClassicPendingScript() {}
 
-NOINLINE void PendingScript::CheckState() const {
+NOINLINE void ClassicPendingScript::CheckState() const {
   // TODO(hiroshige): Turn these CHECK()s into DCHECK() before going to beta.
-  CHECK(is_for_testing_ || element_);
+  CHECK(is_for_testing_ || GetElement());
   CHECK(GetResource() || !streamer_);
   CHECK(!streamer_ || streamer_->GetResource() == GetResource());
 }
 
-void PendingScript::Dispose() {
-  StopWatchingForLoad();
-  DCHECK(!client_);
-  DCHECK(!watching_for_load_);
+NOINLINE void ClassicPendingScript::Dispose() {
+  PendingScript::Dispose();
+}
 
+void ClassicPendingScript::DisposeInternal() {

[haraken, 2017/04/25 17:19:51]

Why can't you call DisposeInternal() before ClassicPendingScript::Dispose()
calls PendingScript::Dispose()?

I'm concerned that this CL is introducing a pretty complicated pre-finalizer
chain. You're doing something like:

1) Call ClassisPendingScript's Dispose()
2) Call PendingScript's Dispose()
3) Let PendingScript's Dispose() call ClassisPendingScript's DisposeInternal()

In the first place, I'm concerned that the resource hierarchy is doing a lot of
complex work in the pre-finalizer. The pre-finalizer is designed to do a couple
of only simple things (e.g., clearing raw pointer). For example, it will cause a
use-after-free bug if the pre-finalizer adds a new reference to an already dead
object (because the marking phase has already completed). Are you pretty sure
that the Resource's pre-finalizer is not doing such a thing?

I'd strongly recommend you refactor the Resource's hierarchy so that it doesn't
heavily rely on the pre-finalizers.

   MemoryCoordinator::Instance().UnregisterClient(this);
   SetResource(nullptr);
-  starting_position_ = TextPosition::BelowRangePosition();
   integrity_failure_ = false;
-  parser_blocking_load_start_time_ = 0;
   if (streamer_)
     streamer_->Cancel();
   streamer_ = nullptr;
-  element_ = nullptr;
 }
 
-void PendingScript::WatchForLoad(PendingScriptClient* client) {
-  CheckState();
-
-  DCHECK(!watching_for_load_);
-  // 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().
-  watching_for_load_ = true;
-  client_ = client;
-  if (IsReady())
-    client_->PendingScriptFinished(this);
-}
-
-void PendingScript::StopWatchingForLoad() {
-  if (!watching_for_load_)
-    return;
+void ClassicPendingScript::StreamingFinished() {
   CheckState();
   DCHECK(GetResource());
-  client_ = nullptr;
-  watching_for_load_ = false;
-}
-
-ScriptElementBase* PendingScript::GetElement() const {
-  // As mentioned in the comment at |m_element| declaration,
-  // |m_element|  must point to the corresponding ScriptLoader's
-  // client.
-  CHECK(element_);
-  return element_.Get();
-}
-
-void PendingScript::StreamingFinished() {
-  CheckState();
-  DCHECK(GetResource());
-  if (client_)
-    client_->PendingScriptFinished(this);
-}
-
-void PendingScript::MarkParserBlockingLoadStartTime() {
-  DCHECK_EQ(parser_blocking_load_start_time_, 0.0);
-  parser_blocking_load_start_time_ = MonotonicallyIncreasingTime();
+  if (Client())
+    Client()->PendingScriptFinished(this);
 }
 
 // Returns true if SRI check passed.
@@ -183,7 +119,7 @@ static bool CheckScriptResourceIntegrity(Resource* resource,
   return true;
 }
 
-void PendingScript::NotifyFinished(Resource* resource) {
+void ClassicPendingScript::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
@@ -207,33 +143,32 @@ void PendingScript::NotifyFinished(Resource* resource) {
   //
   // See https://crbug.com/500701 for more information.
   CheckState();
-  if (element_) {
-    integrity_failure_ = !CheckScriptResourceIntegrity(resource, element_);
+  if (!is_for_testing_ && GetElement()) {
+    integrity_failure_ = !CheckScriptResourceIntegrity(resource, GetElement());
   }
 
   // If script streaming is in use, the client will be notified in
   // streamingFinished.
   if (streamer_)
     streamer_->NotifyFinished(resource);
-  else if (client_)
-    client_->PendingScriptFinished(this);
+  else if (Client())
+    Client()->PendingScriptFinished(this);
 }
 
-void PendingScript::NotifyAppendData(ScriptResource* resource) {
+void ClassicPendingScript::NotifyAppendData(ScriptResource* resource) {
   if (streamer_)
     streamer_->NotifyAppendData(resource);
 }
 
-DEFINE_TRACE(PendingScript) {
-  visitor->Trace(element_);
+DEFINE_TRACE(ClassicPendingScript) {
   visitor->Trace(streamer_);
-  visitor->Trace(client_);
   ResourceOwner<ScriptResource>::Trace(visitor);
   MemoryCoordinatorClient::Trace(visitor);
+  PendingScript::Trace(visitor);
 }
 
-ClassicScript* PendingScript::GetSource(const KURL& document_url,
-                                        bool& error_occurred) const {
+ClassicScript* ClassicPendingScript::GetSource(const KURL& document_url,
+                                               bool& error_occurred) const {
   CheckState();
 
   error_occurred = this->ErrorOccurred();
@@ -245,17 +180,17 @@ ClassicScript* PendingScript::GetSource(const KURL& document_url,
   }
 
   return ClassicScript::Create(ScriptSourceCode(
-      element_->TextContent(), document_url, StartingPosition()));
+      GetElement()->TextContent(), document_url, StartingPosition()));
 }
 
-void PendingScript::SetStreamer(ScriptStreamer* streamer) {
+void ClassicPendingScript::SetStreamer(ScriptStreamer* streamer) {
   DCHECK(!streamer_);
-  DCHECK(!watching_for_load_);
+  DCHECK(!IsWatchingForLoad());
   streamer_ = streamer;
   CheckState();
 }
 
-bool PendingScript::IsReady() const {
+bool ClassicPendingScript::IsReady() const {
   CheckState();
   if (GetResource()) {
     return GetResource()->IsLoaded() && (!streamer_ || streamer_->IsFinished());
@@ -264,7 +199,7 @@ bool PendingScript::IsReady() const {
   return true;
 }
 
-bool PendingScript::ErrorOccurred() const {
+bool ClassicPendingScript::ErrorOccurred() const {
   CheckState();
   if (GetResource())
     return GetResource()->ErrorOccurred() || integrity_failure_;
@@ -272,7 +207,7 @@ bool PendingScript::ErrorOccurred() const {
   return false;
 }
 
-void PendingScript::OnPurgeMemory() {
+void ClassicPendingScript::OnPurgeMemory() {
   CheckState();
   if (!streamer_)
     return;
@@ -280,7 +215,7 @@ void PendingScript::OnPurgeMemory() {
   streamer_ = nullptr;
 }
 
-void PendingScript::StartStreamingIfPossible(
+void ClassicPendingScript::StartStreamingIfPossible(
     Document* document,
     ScriptStreamer::Type streamer_type) {
   if (!document->GetFrame())
@@ -295,4 +230,16 @@ void PendingScript::StartStreamingIfPossible(
       TaskRunnerHelper::Get(TaskType::kNetworking, document));
 }
 
+bool ClassicPendingScript::WasCanceled() const {
+  return GetResource()->WasCanceled();
+}
+
+KURL ClassicPendingScript::Url() const {
+  return GetResource()->Url();
+}
+
+void ClassicPendingScript::RemoveFromMemoryCache() {
+  GetMemoryCache()->Remove(GetResource());
+}
+
 }  // namespace blink