Commit faa05510 authored by hiroshige's avatar hiroshige Committed by Commit bot

Do not call Dispose() as ClassicPendingScript's prefinalizer

Because the things in ClassicPendingScript::DisposeInternal() except for
ScriptStreamer::Cancel() doesn't have to be called as a prefinalizer,
this CL introduces ClassicPendingScript::Prefinalize() that only calls
ScriptStreamer::Cancel() and thus makes Dispose() not to be called there.

This CL simplified the prefinalization of ClassicPendingScript, especially
order dependencies between ClassicPendingScript's prefinalizer and the
prefinalizer of its parent class (ResourceOwner).
Leaving ClassicPendingScript in a not-Dispose()d state is not observable
if the related classes obeys the rule of Oilpan, and
https://codereview.chromium.org/2837413002/ checks that in case there were
a bug.

BUG=715309

Review-Url: https://codereview.chromium.org/2844583002
Cr-Commit-Position: refs/heads/master@{#467299}
parent e9f7bc11
......@@ -47,8 +47,11 @@ NOINLINE void ClassicPendingScript::CheckState() const {
CHECK(!streamer_ || streamer_->GetResource() == GetResource());
}
NOINLINE void ClassicPendingScript::Dispose() {
PendingScript::Dispose();
void ClassicPendingScript::Prefinalize() {
// TODO(hiroshige): Consider moving this to ScriptStreamer's prefinalizer.
// https://crbug.com/715309
if (streamer_)
streamer_->Cancel();
prefinalizer_called_ = true;
}
......
......@@ -27,11 +27,7 @@ class CORE_EXPORT ClassicPendingScript final
public ResourceOwner<ScriptResource>,
public MemoryCoordinatorClient {
USING_GARBAGE_COLLECTED_MIXIN(ClassicPendingScript);
// In order to call Dispose() before ResourceOwner's prefinalizer, we
// also register ClassicPendingScript::Dispose() as the prefinalizer of
// ClassicPendingScript here. https://crbug.com/711703
USING_PRE_FINALIZER(ClassicPendingScript, Dispose);
USING_PRE_FINALIZER(ClassicPendingScript, Prefinalize);
public:
// For script from an external file.
......@@ -61,11 +57,7 @@ class CORE_EXPORT ClassicPendingScript final
void RemoveFromMemoryCache() override;
void DisposeInternal() override;
// Just used as the prefinalizer, does the same as PendingScript::Dispose().
// We define Dispose() with NOINLINE in ClassicPendingScript just to make
// the prefinalizers of PendingScript and ClassicPendingScript have
// different addresses to avoid assertion failures on Windows test bots.
void Dispose();
void Prefinalize();
private:
ClassicPendingScript(ScriptElementBase*,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment