Make BlinkLeakDetector hold the list of ResourceFetcher to stop keepalive loaders

This CL makes BlinkLeakDetector hold the list of all the ResourceFethcers so that the keepalive loaders, which may persist over page navigation, would be cancelled in PrepareForLeakDetection.

Change-Id: I3f7dffa5e549835b496d131fb95b71b5c3e7f035
Reviewed-on: https://chromium-review.googlesource.com/956651
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Keishi Hattori <keishi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543304}

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

@@ -182,6 +182,7 @@
 #include "core/layout/LayoutEmbeddedContent.h"
 #include "core/layout/LayoutView.h"
 #include "core/layout/TextAutosizer.h"
+#include "core/leak_detector/BlinkLeakDetector.h"
 #include "core/loader/CookieJar.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/FrameFetchContext.h"
@@ -690,6 +691,7 @@ Document::Document(const DocumentInit& initializer,
     fetcher_ = FrameFetchContext::CreateFetcherFromDocument(this);
   } else {
     fetcher_ = ResourceFetcher::Create(nullptr);
+    BlinkLeakDetector::Instance().RegisterResourceFetcher(fetcher_);
   }
   DCHECK(fetcher_);
 

third_party/WebKit/Source/core/inspector/InspectorMemoryAgent.cpp

@@ -53,7 +53,7 @@ using PrepareForLeakDetectionCallback =
 using protocol::Response;
 
 InspectorMemoryAgent::InspectorMemoryAgent(InspectedFrames* inspected_frames)
-    : detector_(nullptr), callback_(nullptr), frames_(inspected_frames) {}
+    : callback_(nullptr), frames_(inspected_frames) {}
 
 InspectorMemoryAgent::~InspectorMemoryAgent() = default;
 
@@ -71,16 +71,20 @@ Response InspectorMemoryAgent::getDOMCounters(int* documents,
 void InspectorMemoryAgent::prepareForLeakDetection(
     std::unique_ptr<PrepareForLeakDetectionCallback> callback) {
   callback_ = std::move(callback);
-  detector_.reset(new BlinkLeakDetector(this));
-  detector_->PrepareForLeakDetection(frames_->Root()->Client()->GetWebFrame());
-  detector_->CollectGarbage();
+
+  BlinkLeakDetector& detector = BlinkLeakDetector::Instance();
+  detector.SetClient(this);
+  detector.PrepareForLeakDetection(frames_->Root()->Client()->GetWebFrame());
+  detector.CollectGarbage();
 }
 
 void InspectorMemoryAgent::OnLeakDetectionComplete() {
   DCHECK(callback_);
   callback_->sendSuccess();
   callback_.reset();
-  detector_.reset();
+
+  // Reset the client for BlinkLeakDetector
+  BlinkLeakDetector::Instance().SetClient(nullptr);
 }
 
 void InspectorMemoryAgent::Trace(blink::Visitor* visitor) {

third_party/WebKit/Source/core/inspector/InspectorMemoryAgent.h

@@ -80,7 +80,6 @@ class CORE_EXPORT InspectorMemoryAgent final
   std::unique_ptr<protocol::Memory::SamplingProfile> GetSamplingProfileById(
       uint32_t id);
 
-  std::unique_ptr<BlinkLeakDetector> detector_;
   std::unique_ptr<PrepareForLeakDetectionCallback> callback_;
   Member<InspectedFrames> frames_;
   uint32_t profile_id_ = 0;

third_party/WebKit/Source/core/leak_detector/BlinkLeakDetector.cpp

@@ -17,16 +17,22 @@
 #include "platform/Timer.h"
 #include "platform/bindings/V8PerIsolateData.h"
 #include "platform/loader/fetch/MemoryCache.h"
+#include "platform/loader/fetch/ResourceFetcher.h"
 #include "public/platform/Platform.h"
 
 namespace blink {
 
-BlinkLeakDetector::BlinkLeakDetector(BlinkLeakDetectorClient* client)
+BlinkLeakDetector& BlinkLeakDetector::Instance() {
+  DEFINE_THREAD_SAFE_STATIC_LOCAL(BlinkLeakDetector, blink_leak_detector, ());
+  return blink_leak_detector;
+}
+
+BlinkLeakDetector::BlinkLeakDetector()
     : delayed_gc_timer_(Platform::Current()->CurrentThread()->GetTaskRunner(),
                         this,
                         &BlinkLeakDetector::TimerFiredGC),
       number_of_gc_needed_(0),
-      client_(client) {}
+      client_(nullptr) {}
 
 BlinkLeakDetector::~BlinkLeakDetector() = default;
 
@@ -64,6 +70,10 @@ void BlinkLeakDetector::PrepareForLeakDetection(WebFrame* frame) {
 
   // Clear lazily loaded style sheets.
   CSSDefaultStyleSheets::Instance().PrepareForLeakDetection();
+
+  // Stop keepalive loaders that may persist after page navigation.
+  for (auto resource_fetcher : resource_fetchers_)
+    resource_fetcher->PrepareForLeakDetection();
 }
 
 void BlinkLeakDetector::CollectGarbage() {
@@ -111,4 +121,13 @@ void BlinkLeakDetector::TimerFiredGC(TimerBase*) {
   // Note: Oilpan precise GC is scheduled at the end of the event loop.
 }
 
+void BlinkLeakDetector::SetClient(BlinkLeakDetectorClient* client) {
+  client_ = client;
+}
+
+void BlinkLeakDetector::RegisterResourceFetcher(ResourceFetcher* fetcher) {
+  DCHECK(IsMainThread());
+  resource_fetchers_.insert(fetcher);
+}
+
 }  // namespace blink

third_party/WebKit/Source/core/leak_detector/BlinkLeakDetector.h

@@ -12,24 +12,34 @@ namespace blink {
 
 class BlinkLeakDetectorClient;
 class WebFrame;
+class ResourceFetcher;
 
 // This class is responsible for stabilizing the detection results which are
 // InstanceCounter values by 1) preparing for leak detection and 2) operating
 // garbage collections before leak detection.
+// This class is implemented as singleton, but there needs to be only one user
+// at a time.
 class CORE_EXPORT BlinkLeakDetector {
  public:
-  explicit BlinkLeakDetector(BlinkLeakDetectorClient*);
+  static BlinkLeakDetector& Instance();
   ~BlinkLeakDetector();
 
   void PrepareForLeakDetection(WebFrame*);
   void CollectGarbage();
+  void SetClient(BlinkLeakDetectorClient*);
+
+  // This registration function needs to be called whenever ResourceFetcher is
+  // created (i.e. ResourceFetcher::Create is called).
+  void RegisterResourceFetcher(ResourceFetcher*);
 
  private:
+  BlinkLeakDetector();
   void TimerFiredGC(TimerBase*);
 
   TaskRunnerTimer<BlinkLeakDetector> delayed_gc_timer_;
   int number_of_gc_needed_;
   BlinkLeakDetectorClient* client_;
+  PersistentHeapHashSet<WeakMember<ResourceFetcher>> resource_fetchers_;
 };
 }  // namespace blink
 

third_party/WebKit/Source/core/leak_detector/WebLeakDetector.cpp

@@ -46,7 +46,7 @@ class WebLeakDetectorImpl final : public WebLeakDetector,
 
  public:
   explicit WebLeakDetectorImpl(WebLeakDetectorClient* client)
-      : client_(client), detector_(nullptr) {
+      : client_(client) {
     DCHECK(client_);
   }
 
@@ -60,17 +60,17 @@ class WebLeakDetectorImpl final : public WebLeakDetector,
 
  private:
   WebLeakDetectorClient* client_;
-  std::unique_ptr<BlinkLeakDetector> detector_;
   DISALLOW_COPY_AND_ASSIGN(WebLeakDetectorImpl);
 };
 
 void WebLeakDetectorImpl::PrepareForLeakDetection(WebFrame* frame) {
-  detector_.reset(new BlinkLeakDetector(this));
-  detector_->PrepareForLeakDetection(frame);
+  BlinkLeakDetector& detector = BlinkLeakDetector::Instance();
+  detector.SetClient(this);
+  detector.PrepareForLeakDetection(frame);
 }
 
 void WebLeakDetectorImpl::CollectGarbageAndReport() {
-  detector_->CollectGarbage();
+  BlinkLeakDetector::Instance().CollectGarbage();
 }
 
 void WebLeakDetectorImpl::OnLeakDetectionComplete() {
@@ -101,7 +101,8 @@ void WebLeakDetectorImpl::OnLeakDetectionComplete() {
       InstanceCounters::CounterValue(InstanceCounters::kUACSSResourceCounter);
 
   client_->OnLeakDetectionComplete(result);
-  detector_.reset();
+  // Reset the client for BlinkLeakDetector
+  BlinkLeakDetector::Instance().SetClient(nullptr);
 
 #ifndef NDEBUG
   showLiveDocumentInstances();

third_party/WebKit/Source/core/loader/FrameFetchContext.cpp

@@ -49,6 +49,7 @@
 #include "core/inspector/ConsoleMessage.h"
 #include "core/inspector/IdentifiersFactory.h"
 #include "core/inspector/InspectorTraceEvents.h"
+#include "core/leak_detector/BlinkLeakDetector.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/IdlenessDetector.h"
@@ -252,6 +253,7 @@ ResourceFetcher* FrameFetchContext::CreateFetcher(DocumentLoader* loader,
                                                   Document* document) {
   FrameFetchContext* context = new FrameFetchContext(loader, document);
   ResourceFetcher* fetcher = ResourceFetcher::Create(context);
+  BlinkLeakDetector::Instance().RegisterResourceFetcher(fetcher);
 
   if (loader && context->GetSettings()->GetSavePreviousDocumentResources() !=
                     SavePreviousDocumentResources::kNever) {

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp

@@ -1640,6 +1640,12 @@ void ResourceFetcher::EmulateLoadStartedForInspector(
   RequestLoadStarted(resource->Identifier(), resource, params, kUse);
 }
 
+void ResourceFetcher::PrepareForLeakDetection() {
+  // Stop loaders including keepalive ones that may persist after page
+  // navigation and thus affect instance counters of leak detection.
+  StopFetchingIncludingKeepaliveLoaders();
+}
+
 void ResourceFetcher::StopFetchingInternal(StopFetchingTarget target) {
   // TODO(toyoshim): May want to suspend scheduler while canceling loaders so
   // that the cancellations below do not awake unnecessary scheduling.

third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.h

@@ -171,6 +171,11 @@ class PLATFORM_EXPORT ResourceFetcher
                                       WebURLRequest::RequestContext,
                                       const AtomicString& initiator_name);
 
+  // This is called from leak detectors (Real-world leak detector & layout test
+  // leak detector) to clean up loaders after page navigation before instance
+  // counting.
+  void PrepareForLeakDetection();
+
  private:
   friend class ResourceCacheValidationSuppressor;
   enum class StopFetchingTarget {