Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(179)

Unified Diff: third_party/WebKit/Source/core/dom/PendingScript.cpp

Issue 2828193003: Explicitly track the lifecycle of PendingScript. (Closed)
Patch Set: document, and replace watching_for_load_ Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/core/dom/PendingScript.cpp
diff --git a/third_party/WebKit/Source/core/dom/PendingScript.cpp b/third_party/WebKit/Source/core/dom/PendingScript.cpp
index c3471192149d5137d2587196893412edf0d0d8b1..e5e7341e991f9768b1f6cf59e9a5d6a2843dde1a 100644
--- a/third_party/WebKit/Source/core/dom/PendingScript.cpp
+++ b/third_party/WebKit/Source/core/dom/PendingScript.cpp
@@ -57,7 +57,7 @@ PendingScript::PendingScript(ScriptElementBase* element,
ScriptResource* resource,
const TextPosition& starting_position,
bool is_for_testing)
- : watching_for_load_(false),
+ : ready_state_(resource ? kWaitingForResource : kReady),
element_(element),
starting_position_(starting_position),
integrity_failure_(false),
@@ -80,8 +80,7 @@ NOINLINE void PendingScript::CheckState() const {
void PendingScript::Dispose() {
StopWatchingForLoad();
- DCHECK(!client_);
- DCHECK(!watching_for_load_);
+ DCHECK(!WatchingForLoad());
MemoryCoordinator::Instance().UnregisterClient(this);
SetResource(nullptr);
@@ -97,25 +96,24 @@ void PendingScript::Dispose() {
void PendingScript::WatchForLoad(PendingScriptClient* client) {
CheckState();
- DCHECK(!watching_for_load_);
+ DCHECK(!WatchingForLoad());
+ DCHECK(client);
// 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_)
+ if (!WatchingForLoad())
return;
CheckState();
DCHECK(GetResource());
client_ = nullptr;
- watching_for_load_ = false;
}
ScriptElementBase* PendingScript::GetElement() const {
@@ -129,8 +127,9 @@ ScriptElementBase* PendingScript::GetElement() const {
void PendingScript::StreamingFinished() {
CheckState();
DCHECK(GetResource());
hiroshige 2017/04/24 19:16:53 DCHECK_EQ(ready_state_, kWaitingForStreaming);
jbroman 2017/05/01 15:08:01 Done.
- if (client_)
- client_->PendingScriptFinished(this);
+
+ bool error_occurred = GetResource()->ErrorOccurred() || integrity_failure_;
+ AdvanceReadyState(error_occurred ? kErrorOccurred : kReady);
}
void PendingScript::MarkParserBlockingLoadStartTime() {
@@ -184,6 +183,8 @@ static bool CheckScriptResourceIntegrity(Resource* resource,
}
void PendingScript::NotifyFinished(Resource* resource) {
+ DCHECK_EQ(ready_state_, kWaitingForResource);
+
// 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
@@ -211,12 +212,13 @@ void PendingScript::NotifyFinished(Resource* resource) {
integrity_failure_ = !CheckScriptResourceIntegrity(resource, element_);
}
- // If script streaming is in use, the client will be notified in
- // streamingFinished.
+ // We are now waiting for script streaming to finish.
+ // If there is no script streamer, this step completes immediately.
+ AdvanceReadyState(kWaitingForStreaming);
if (streamer_)
streamer_->NotifyFinished(resource);
- else if (client_)
- client_->PendingScriptFinished(this);
+ else
+ StreamingFinished();
}
void PendingScript::NotifyAppendData(ScriptResource* resource) {
@@ -235,8 +237,9 @@ DEFINE_TRACE(PendingScript) {
ClassicScript* PendingScript::GetSource(const KURL& document_url,
bool& error_occurred) const {
CheckState();
+ DCHECK(IsReady());
- error_occurred = this->ErrorOccurred();
+ error_occurred = ErrorOccurred();
if (GetResource()) {
DCHECK(GetResource()->IsLoaded());
if (streamer_ && !streamer_->StreamingSuppressed())
@@ -250,28 +253,13 @@ ClassicScript* PendingScript::GetSource(const KURL& document_url,
void PendingScript::SetStreamer(ScriptStreamer* streamer) {
DCHECK(!streamer_);
- DCHECK(!watching_for_load_);
+ DCHECK(!WatchingForLoad());
+ DCHECK(!streamer->IsFinished());
+ DCHECK_LT(ready_state_, kWaitingForStreaming);
streamer_ = streamer;
CheckState();
}
-bool PendingScript::IsReady() const {
- CheckState();
- if (GetResource()) {
- return GetResource()->IsLoaded() && (!streamer_ || streamer_->IsFinished());
- }
-
- return true;
-}
-
-bool PendingScript::ErrorOccurred() const {
- CheckState();
- if (GetResource())
- return GetResource()->ErrorOccurred() || integrity_failure_;
-
- return false;
-}
-
void PendingScript::OnPurgeMemory() {
CheckState();
if (!streamer_)
@@ -295,4 +283,17 @@ void PendingScript::StartStreamingIfPossible(
TaskRunnerHelper::Get(TaskType::kNetworking, document));
}
+void PendingScript::AdvanceReadyState(ReadyState ready_state) {
+ // The ready state should monotonically advance.
+ DCHECK_GT(ready_state, ready_state_);
+
+ // The state should not advance from one completed state to another.
+ if (ready_state >= kReady)
+ DCHECK_LT(ready_state_, kReady);
+
+ ready_state_ = ready_state;
+ if (ready_state >= kReady && client_)
+ client_->PendingScriptFinished(this);
+}
+
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698