Scheduler: Deprecate CancellableTaskFactory in favor of WebTaskRunner::postCancellableTask (2)

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

Index: third_party/WebKit/Source/core/dom/Document.cpp
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
index cd62e65908154b0f7b9b74d89fec2e3f860e652c..a0a49654aed5a9f85546217178bc1feedb54d138 100644
--- a/third_party/WebKit/Source/core/dom/Document.cpp
+++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -234,7 +234,6 @@
 #include "platform/ScriptForbiddenScope.h"
 #include "platform/network/ContentSecurityPolicyParsers.h"
 #include "platform/network/HTTPParsers.h"
-#include "platform/scheduler/CancellableTaskFactory.h"
 #include "platform/scroll/ScrollbarTheme.h"
 #include "platform/text/PlatformLocale.h"
 #include "platform/text/SegmentedString.h"
@@ -428,9 +427,6 @@ Document::Document(const DocumentInit& initializer,
       m_paginatedForScreen(false),
       m_compatibilityMode(NoQuirksMode),
       m_compatibilityModeLocked(false),
-      m_executeScriptsWaitingForResourcesTask(CancellableTaskFactory::create(
-          this,
-          &Document::executeScriptsWaitingForResources)),
       m_hasAutofocused(false),
       m_clearFocusedElementTimer(
           TaskRunnerHelper::get(TaskType::Internal, this),
@@ -3268,9 +3264,14 @@ void Document::didRemoveAllPendingStylesheet() {
 }
 
 void Document::didLoadAllScriptBlockingResources() {
-  TaskRunnerHelper::get(TaskType::Networking, this)
-      ->postTask(BLINK_FROM_HERE,
-                 m_executeScriptsWaitingForResourcesTask->cancelAndCreate());
+  // wrapWeakPersistent(this) is safe because a posted task is canceled when the
+  // task handle is destroyed on the dtor of this Document.

[haraken, 2016/11/16 11:37:20]

tzik@: Should we use wrapWeakPersistent? Or wrapPersistent?

Regarding overhead, wrapPersistent is smaller than wrapWeakPersistent since it
doesn't require weak processing.

[tzik, 2016/11/17 06:20:14]

That depends on the context: We should use wrapPersistent if we want to keep
|this| alive and run the task even after all other references to |this| have
gone. Otherwise, wrapWeakPersistent.
In this case, IIUC, we should not keep Document alive just for the script
execution. So wrapWeakPersistent should fit here.

[haraken, 2016/11/17 06:35:19]

Makes sense.

[nhiroki, 2016/11/17 08:10:15]

Thank you for the clarification. Added the comment.

+  m_executeScriptsWaitingForResourcesTaskHandle =
+      TaskRunnerHelper::get(TaskType::Networking, this)
+          ->postCancellableTask(
+              BLINK_FROM_HERE,
+              WTF::bind(&Document::executeScriptsWaitingForResources,
+                        wrapWeakPersistent(this)));
 
   if (isHTMLDocument() && body()) {
     // For HTML if we have no more stylesheets to load and we're past the body