Commit d6b07146 authored by Dave Tapuska's avatar Dave Tapuska Committed by Commit Bot

Pausable Object Cleanup - PausableTask

Make PausableTask not inherit PausableTimer. There is no need
to schedule a timer since the kJavascriptTimer task queue already
is pausable. This class has no real user yet; Devlin claims that the
extension code will be using it soon.

BUG=907125

Change-Id: I270ef5d8e7f67c32d2377d8f8785ec56e941c204
Reviewed-on: https://chromium-review.googlesource.com/c/1378233Reviewed-by: default avatarAlexander Timin <altimin@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Commit-Queue: Dave Tapuska <dtapuska@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622083}
parent f101252e
...@@ -638,23 +638,22 @@ TEST_F(WebFrameTest, CallingPostPausableTaskWhilePaused) ...@@ -638,23 +638,22 @@ TEST_F(WebFrameTest, CallingPostPausableTaskWhilePaused)
ScriptState::Scope scope(ToScriptStateForMainWorld(main_frame->GetFrame())); ScriptState::Scope scope(ToScriptStateForMainWorld(main_frame->GetFrame()));
// Suspend scheduled tasks so the script doesn't run. // Suspend scheduled tasks so the script doesn't run.
main_frame->GetFrame()->GetDocument()->PauseScheduledTasks(); web_view_helper.GetWebView()->GetPage()->SetPaused(true);
ScriptNotPausedCallbackHelper callback_helper; ScriptNotPausedCallbackHelper callback_helper;
main_frame->PostPausableTask(callback_helper.GetCallback()); main_frame->PostPausableTask(callback_helper.GetCallback());
RunPendingTasks(); RunPendingTasks();
EXPECT_FALSE(callback_helper.result()); EXPECT_FALSE(callback_helper.result());
main_frame->GetFrame()->GetDocument()->UnpauseScheduledTasks(); web_view_helper.GetWebView()->GetPage()->SetPaused(false);
RunPendingTasks(); RunPendingTasks();
ASSERT_TRUE(callback_helper.result()); ASSERT_TRUE(callback_helper.result());
EXPECT_EQ(WebLocalFrame::PausableTaskResult::kReady, EXPECT_EQ(WebLocalFrame::PausableTaskResult::kReady,
*callback_helper.result()); *callback_helper.result());
} }
TEST_F(WebFrameTest, CallingPostPausableTaskAndNavigating) { TEST_F(WebFrameTest, CallingPostPausableTaskAndDestroying) {
RegisterMockedHttpURLLoad("foo.html"); RegisterMockedHttpURLLoad("foo.html");
RegisterMockedHttpURLLoad("bar.html");
frame_test_helpers::WebViewHelper web_view_helper; frame_test_helpers::WebViewHelper web_view_helper;
web_view_helper.InitializeAndLoad(base_url_ + "foo.html"); web_view_helper.InitializeAndLoad(base_url_ + "foo.html");
...@@ -663,17 +662,14 @@ TEST_F(WebFrameTest, CallingPostPausableTaskAndNavigating) { ...@@ -663,17 +662,14 @@ TEST_F(WebFrameTest, CallingPostPausableTaskAndNavigating) {
ScriptState::Scope scope(ToScriptStateForMainWorld(main_frame->GetFrame())); ScriptState::Scope scope(ToScriptStateForMainWorld(main_frame->GetFrame()));
// Suspend scheduled tasks so the script doesn't run. // Suspend scheduled tasks so the script doesn't run.
main_frame->GetFrame()->GetDocument()->PauseScheduledTasks(); web_view_helper.GetWebView()->GetPage()->SetPaused(true);
ScriptNotPausedCallbackHelper callback_helper; ScriptNotPausedCallbackHelper callback_helper;
main_frame->PostPausableTask(callback_helper.GetCallback()); main_frame->PostPausableTask(callback_helper.GetCallback());
RunPendingTasks(); RunPendingTasks();
EXPECT_FALSE(callback_helper.result()); EXPECT_FALSE(callback_helper.result());
// If the frame navigates, pending scripts should be removed, but the callback web_view_helper.Reset();
// should always be ran.
frame_test_helpers::LoadFrame(web_view_helper.GetWebView()->MainFrameImpl(),
base_url_ + "bar.html");
ASSERT_TRUE(callback_helper.result()); ASSERT_TRUE(callback_helper.result());
EXPECT_EQ(WebLocalFrame::PausableTaskResult::kContextInvalidOrDestroyed, EXPECT_EQ(WebLocalFrame::PausableTaskResult::kContextInvalidOrDestroyed,
*callback_helper.result()); *callback_helper.result());
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include "base/location.h" #include "base/location.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/platform/wtf/functional.h"
namespace blink { namespace blink {
...@@ -25,7 +26,6 @@ void PausableTask::Post(ExecutionContext* context, ...@@ -25,7 +26,6 @@ void PausableTask::Post(ExecutionContext* context,
} }
void PausableTask::ContextDestroyed(ExecutionContext* destroyed_context) { void PausableTask::ContextDestroyed(ExecutionContext* destroyed_context) {
PausableTimer::ContextDestroyed(destroyed_context);
DCHECK(callback_); DCHECK(callback_);
Dispose(); Dispose();
...@@ -34,7 +34,7 @@ void PausableTask::ContextDestroyed(ExecutionContext* destroyed_context) { ...@@ -34,7 +34,7 @@ void PausableTask::ContextDestroyed(ExecutionContext* destroyed_context) {
WebLocalFrame::PausableTaskResult::kContextInvalidOrDestroyed); WebLocalFrame::PausableTaskResult::kContextInvalidOrDestroyed);
} }
void PausableTask::Fired() { void PausableTask::Run() {
CHECK(!GetExecutionContext()->IsContextDestroyed()); CHECK(!GetExecutionContext()->IsContextDestroyed());
DCHECK(!GetExecutionContext()->IsContextPaused()); DCHECK(!GetExecutionContext()->IsContextPaused());
DCHECK(callback_); DCHECK(callback_);
...@@ -50,23 +50,23 @@ void PausableTask::Fired() { ...@@ -50,23 +50,23 @@ void PausableTask::Fired() {
PausableTask::PausableTask(ExecutionContext* context, PausableTask::PausableTask(ExecutionContext* context,
WebLocalFrame::PausableTaskCallback callback) WebLocalFrame::PausableTaskCallback callback)
: PausableTimer(context, TaskType::kJavascriptTimer), : ContextLifecycleObserver(context),
callback_(std::move(callback)), callback_(std::move(callback)),
keep_alive_(this) { keep_alive_(this) {
DCHECK(callback_); DCHECK(callback_);
DCHECK(context); DCHECK(context);
DCHECK(!context->IsContextDestroyed()); DCHECK(!context->IsContextDestroyed());
DCHECK(context->IsContextPaused()); DCHECK(context->IsContextPaused());
task_handle_ = PostCancellableTask(
StartOneShot(TimeDelta(), FROM_HERE); *context->GetTaskRunner(TaskType::kInternalDefault), FROM_HERE,
PauseIfNeeded(); WTF::Bind(&PausableTask::Run, WrapPersistent(this)));
} }
void PausableTask::Dispose() { void PausableTask::Dispose() {
// Remove object as a ContextLifecycleObserver. // Remove object as a ContextLifecycleObserver.
PausableObject::ClearContext(); ContextLifecycleObserver::ClearContext();
keep_alive_.Clear(); keep_alive_.Clear();
Stop(); task_handle_.Cancel();
} }
} // namespace blink } // namespace blink
...@@ -9,8 +9,9 @@ ...@@ -9,8 +9,9 @@
#include "third_party/blink/public/web/web_local_frame.h" #include "third_party/blink/public/web/web_local_frame.h"
#include "third_party/blink/renderer/core/core_export.h" #include "third_party/blink/renderer/core/core_export.h"
#include "third_party/blink/renderer/core/frame/pausable_timer.h" #include "third_party/blink/renderer/core/dom/context_lifecycle_observer.h"
#include "third_party/blink/renderer/platform/heap/self_keep_alive.h" #include "third_party/blink/renderer/platform/heap/self_keep_alive.h"
#include "third_party/blink/renderer/platform/scheduler/public/post_cancellable_task.h"
namespace blink { namespace blink {
...@@ -19,13 +20,13 @@ namespace blink { ...@@ -19,13 +20,13 @@ namespace blink {
// invalidated. // invalidated.
class CORE_EXPORT PausableTask final class CORE_EXPORT PausableTask final
: public GarbageCollectedFinalized<PausableTask>, : public GarbageCollectedFinalized<PausableTask>,
public PausableTimer { public ContextLifecycleObserver {
USING_GARBAGE_COLLECTED_MIXIN(PausableTask); USING_GARBAGE_COLLECTED_MIXIN(PausableTask);
public: public:
// Note: This asserts that the context is currently suspended. // Note: This asserts that the context is currently suspended.
PausableTask(ExecutionContext*, WebLocalFrame::PausableTaskCallback); PausableTask(ExecutionContext*, WebLocalFrame::PausableTaskCallback);
~PausableTask() override; virtual ~PausableTask();
// Checks if the context is paused, and, if not, executes the callback // Checks if the context is paused, and, if not, executes the callback
// immediately. Otherwise constructs a PausableTask that will run the // immediately. Otherwise constructs a PausableTask that will run the
...@@ -34,12 +35,13 @@ class CORE_EXPORT PausableTask final ...@@ -34,12 +35,13 @@ class CORE_EXPORT PausableTask final
// PausableTimer: // PausableTimer:
void ContextDestroyed(ExecutionContext*) override; void ContextDestroyed(ExecutionContext*) override;
void Fired() override;
private: private:
void Run();
void Dispose(); void Dispose();
WebLocalFrame::PausableTaskCallback callback_; WebLocalFrame::PausableTaskCallback callback_;
TaskHandle task_handle_;
SelfKeepAlive<PausableTask> keep_alive_; SelfKeepAlive<PausableTask> keep_alive_;
}; };
......
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