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

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

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2011, 2012 Apple Inc. All
  * rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008, 2009, 2011, 2012 Google Inc. All rights reserved.
@@ skipping 216 matching lines @@
 #include "platform/EventDispatchForbiddenScope.h"
 #include "platform/Histogram.h"
 #include "platform/InstanceCounters.h"
 #include "platform/Language.h"
 #include "platform/LengthFunctions.h"
 #include "platform/PluginScriptForbiddenScope.h"
 #include "platform/RuntimeEnabledFeatures.h"
 #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"
 #include "platform/tracing/TraceEvent.h"
 #include "platform/weborigin/OriginAccessEntry.h"
 #include "platform/weborigin/SchemeRegistry.h"
 #include "platform/weborigin/SecurityOrigin.h"
 #include "public/platform/Platform.h"
 #include "public/platform/WebAddressSpace.h"
 #include "public/platform/WebFrameScheduler.h"
@@ skipping 173 matching lines @@
       m_frame(initializer.frame()),
       m_domWindow(m_frame ? m_frame->localDOMWindow() : 0),
       m_importsController(this, initializer.importsController()),
       m_contextFeatures(ContextFeatures::defaultSwitch()),
       m_wellFormed(false),
       m_implementation(this, nullptr),
       m_printing(NotPrinting),
       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),
           this,
           &Document::clearFocusedElementTimerFired),
       m_domTreeVersion(++s_globalTreeVersion),
       m_styleVersion(0),
       m_listenerTypes(0),
       m_mutationObserverTypes(0),
       m_styleEngine(this, nullptr),
@@ skipping 2817 matching lines @@
 
   // Only imports on master documents can trigger rendering.
   if (HTMLImportLoader* import = importLoader())
     import->didRemoveAllPendingStylesheet();
   if (!haveImportsLoaded())
     return;
   didLoadAllScriptBlockingResources();
 }
 
 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
     // tag, we should have something to paint so resume.
     beginLifecycleUpdatesIfRenderingReady();
   } else if (!isHTMLDocument() && documentElement()) {
     // For non-HTML there is no body so resume as soon as the sheets are loaded.
     beginLifecycleUpdatesIfRenderingReady();
   }
 
@@ skipping 3152 matching lines @@
 }
 
 void showLiveDocumentInstances() {
   WeakDocumentSet& set = liveDocumentSet();
   fprintf(stderr, "There are %u documents currently alive:\n", set.size());
   for (Document* document : set)
     fprintf(stderr, "- Document %p URL: %s\n", document,
             document->url().getString().utf8().data());
 }
 #endif