Make NavigationScheduler post loading tasks instead of timers

Source/core/loader/NavigationScheduler.cpp

Index: Source/core/loader/NavigationScheduler.cpp
diff --git a/Source/core/loader/NavigationScheduler.cpp b/Source/core/loader/NavigationScheduler.cpp
index 2841a7980c71c1b8148245126de3899c5819f461..fb383ba36357f39070c5ddb80384382b3addf2c4 100644
--- a/Source/core/loader/NavigationScheduler.cpp
+++ b/Source/core/loader/NavigationScheduler.cpp
@@ -48,6 +48,7 @@
 #include "core/page/Page.h"
 #include "platform/SharedBuffer.h"
 #include "platform/UserGestureIndicator.h"
+#include "public/platform/Platform.h"
 #include "wtf/CurrentTime.h"
 
 namespace blink {
@@ -266,7 +267,7 @@ private:
 
 NavigationScheduler::NavigationScheduler(LocalFrame* frame)
     : m_frame(frame)
-    , m_timer(this, &NavigationScheduler::timerFired)
+    , m_navigateTaskFactory(WTF::bind(&NavigationScheduler::navigateTask, this))

[haraken, 2015/09/03 23:43:30]

Sigbjorn: NavigationScheduler is a part of object. What happens if we create a
closure that points to a part of object? (i.e., how does the closure keep it
alive?)

[sof, 2015/09/04 06:28:34]

It will work out fine, but https://codereview.chromium.org/1312843009/ makes
these task factories no longer be part objects.

If the task (CancellableTask, in this case) has a weak pointer back to its
factory (the CancellableTaskFactory part object), then it doesn't matter where
the factory object is allocated -- once it is destructed, the weak pointers are
revoked & the task will no longer be able to access the closure.

(cf. CancellableTaskFactory::CancellableTask::run().)

[haraken, 2015/09/04 06:32:40]

Makes sense.

     , m_navigationDisableCount(0)
 {
 }
@@ -371,7 +372,7 @@ void NavigationScheduler::scheduleReload()
     schedule(ScheduledReload::create());
 }
 
-void NavigationScheduler::timerFired(Timer<NavigationScheduler>*)
+void NavigationScheduler::navigateTask()
 {
     if (!m_frame->page())
         return;
@@ -413,20 +414,22 @@ void NavigationScheduler::startTimer()
         return;
 
     ASSERT(m_frame->page());
-    if (m_timer.isActive())
+    if (m_navigateTaskFactory.isPending())
         return;
     if (!m_redirect->shouldStartTimer(m_frame))
         return;
 
-    m_timer.startOneShot(m_redirect->delay(), FROM_HERE);
+    Platform::current()->currentThread()->scheduler()->loadingTaskRunner()->postDelayedTask(
+        FROM_HERE, m_navigateTaskFactory.cancelAndCreate(), m_redirect->delay());

[jochen (gone - plz use gerrit), 2015/09/07 14:50:59]

this is wrong: startOneShot takes seconds, postDelayedTask takes milliseconds.

+
     InspectorInstrumentation::frameScheduledNavigation(m_frame, m_redirect->delay());
 }
 
 void NavigationScheduler::cancel()
 {
-    if (m_timer.isActive())
+    if (m_navigateTaskFactory.isPending())
         InspectorInstrumentation::frameClearedScheduledNavigation(m_frame);
-    m_timer.stop();
+    m_navigateTaskFactory.cancel();
     m_redirect.clear();
 }