Move ThreadableLoader to Oilpan heap (3/3)

third_party/WebKit/Source/core/loader/WorkerThreadableLoader.h

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 37 matching lines @@
 namespace blink {
 
 class ResourceError;
 class ResourceRequest;
 class ResourceResponse;
 class WorkerGlobalScope;
 class WorkerLoaderProxy;
 struct CrossThreadResourceRequestData;
 struct CrossThreadResourceTimingInfoData;
 
-// TODO(yhirano): Draw a diagram to illustrate the class relationship.
-// TODO(yhirano): Rename inner classes so that readers can see in which thread
-// they are living easily.
+// A WorkerThreadableLoader is a ThreadableLoader implementation intended to
+// be used in a WebWorker thread. Because Blink's ResourceFetcher and
+// ResourceLoader work only in the main thread, A WorkerThreadableLoader holds

[haraken, 2016/07/29 15:00:44]

A => a

[yhirano, 2016/08/01 08:52:38]

Done.

+// a ThreadableLoader in the main thread and delegates tasks asynchronously
+// to the loader.
+//
+// CTP: CrossThreadPersistent
+// CTWP: CrossThreadWeakPersistent
+//
+// ----------------------------------------------------------------
+//                 +------------------------+
+//       raw ptr   | ThreadableLoaderClient |
+//      +--------> | worker thread          |
+//      |          +------------------------+
+//      |
+// +----+------------------+    CTP  +------------------------+
+// + WorkerThreadableLoader|<--------+ MainThreadLoaderHolder |
+// | worker thread         +-------->| main thread            |
+// +-----------------------+   CTWP  +----------------------+-+

[haraken, 2016/07/29 15:00:44]

I asked this question before but let me confirm again: What happens if we use
CTP for the WorkerThreadableLoader => MainThreadLoaderHolder? It will create a
cycle temporarily but the cycle is explicitly broken when the worker thread
dies. So it won't cause memory leaks, right?

[yhirano, 2016/08/01 08:52:38]

It's a bit more complicated.

1. As WorkerThreadableLoader requires users to cancel it appropriately (see
ThreadableLoader's comments), WorkerThreadableLoader <=>
MainThreadableLoaderHelper persistent loop should be no harm (though I don't
feel good with the loop).

2. It is not planned now, but I think it's good to move ThreadableLoaderClient
to the heap. With on-heap ThreadableLoaderClient, WorkerThreadableLoader =>
ThreadableLoaderClient "raw ptr" will be replaced with a Member ptr. Also, it's
very common that a ThreadableLoaderClient has a  ThreadableLoader as Member.
That means there will be a persistent loop from ThreadableLoaderClient to
ThreadableLoaderClient itself. That's problematic. Assume that a
ThreadableLoader has a pre-finalizer that cancels its ThreadableLoader. That
pre-finalizer will not be called because there is a persistent loop.

3. As you say that loop will be broken when the thread is terminated but I think
that's not enough. We still see unbound memory consumption increase in a
long-running worker thread.

+//                                                          |
+//                                 +------------------+     | Member
+//                                 | ThreadableLoader | <---+
+//                                 |      main thread |
+//                                 +------------------+

[haraken, 2016/07/29 15:00:44]

This model looks amazing! It looks much simpler than before.

+//
 class WorkerThreadableLoader final : public ThreadableLoader {
 public:
     static void loadResourceSynchronously(WorkerGlobalScope&, const ResourceRequest&, ThreadableLoaderClient&, const ThreadableLoaderOptions&, const ResourceLoaderOptions&);
     static WorkerThreadableLoader* create(WorkerGlobalScope& workerGlobalScope, ThreadableLoaderClient* client, const ThreadableLoaderOptions& options, const ResourceLoaderOptions& resourceLoaderOptions)
     {
         return new WorkerThreadableLoader(workerGlobalScope, client, options, resourceLoaderOptions, LoadAsynchronously);
     }
 
     ~WorkerThreadableLoader() override;
 
@@ skipping 21 matching lines @@
         DEFINE_INLINE_VIRTUAL_TRACE() {}
     };
     class AsyncTaskForwarder;
     class WaitableEventWithTasks;
     class SyncTaskForwarder;
 
     // An instance of this class lives in the main thread. It is a
     // ThreadableLoaderClient for a DocumentThreadableLoader and forward
     // notifications to the associated WorkerThreadableLoader living in the
     // worker thread.
-    class Peer final : public GarbageCollectedFinalized<Peer>, public ThreadableLoaderClient, public WorkerThreadLifecycleObserver {
-        USING_GARBAGE_COLLECTED_MIXIN(Peer);
+    class MainThreadLoaderHolder final : public GarbageCollectedFinalized<MainThreadLoaderHolder>, public ThreadableLoaderClient, public WorkerThreadLifecycleObserver {
+        USING_GARBAGE_COLLECTED_MIXIN(MainThreadLoaderHolder);
     public:
         static void createAndStart(
             WorkerThreadableLoader*,
             PassRefPtr<WorkerLoaderProxy>,
             WorkerThreadLifecycleContext*,
             std::unique_ptr<CrossThreadResourceRequestData>,
             const ThreadableLoaderOptions&,
             const ResourceLoaderOptions&,
             PassRefPtr<WaitableEventWithTasks>,
             ExecutionContext*);
-        ~Peer() override;
+        ~MainThreadLoaderHolder() override;
 
         void overrideTimeout(unsigned long timeoutMillisecond);
         void cancel();
 
         void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent) override;
         void didReceiveResponse(unsigned long identifier, const ResourceResponse&, std::unique_ptr<WebDataConsumerHandle>) override;
         void didReceiveData(const char*, unsigned dataLength) override;
         void didDownloadData(int dataLength) override;
         void didReceiveCachedMetadata(const char*, int dataLength) override;
         void didFinishLoading(unsigned long identifier, double finishTime) override;
         void didFail(const ResourceError&) override;
         void didFailAccessControlCheck(const ResourceError&) override;
         void didFailRedirectCheck() override;
         void didReceiveResourceTiming(const ResourceTimingInfo&) override;
 
         void contextDestroyed() override;
 
         DECLARE_TRACE();
 
     private:
-        Peer(TaskForwarder*, WorkerThreadLifecycleContext*);
+        MainThreadLoaderHolder(TaskForwarder*, WorkerThreadLifecycleContext*);
         void start(Document&, std::unique_ptr<CrossThreadResourceRequestData>, const ThreadableLoaderOptions&, const ResourceLoaderOptions&);
 
         Member<TaskForwarder> m_forwarder;
         Member<ThreadableLoader> m_mainThreadLoader;
 
         // |*m_workerLoader| lives in the worker thread.
         CrossThreadWeakPersistent<WorkerThreadableLoader> m_workerLoader;
     };
 
     WorkerThreadableLoader(WorkerGlobalScope&, ThreadableLoaderClient*, const ThreadableLoaderOptions&, const ResourceLoaderOptions&, BlockingBehavior);
-    void didStart(Peer*);
+    void didStart(MainThreadLoaderHolder*);
 
     void didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent);
     void didReceiveResponse(unsigned long identifier, std::unique_ptr<CrossThreadResourceResponseData>, std::unique_ptr<WebDataConsumerHandle>);
     void didReceiveData(std::unique_ptr<Vector<char>> data);
     void didReceiveCachedMetadata(std::unique_ptr<Vector<char>> data);
     void didFinishLoading(unsigned long identifier, double finishTime);
     void didFail(const ResourceError&);
     void didFailAccessControlCheck(const ResourceError&);
     void didFailRedirectCheck();
     void didDownloadData(int dataLength);
     void didReceiveResourceTiming(std::unique_ptr<CrossThreadResourceTimingInfoData>);
 
     Member<WorkerGlobalScope> m_workerGlobalScope;
     RefPtr<WorkerLoaderProxy> m_workerLoaderProxy;
     ThreadableLoaderClient* m_client;
 
     ThreadableLoaderOptions m_threadableLoaderOptions;
     ResourceLoaderOptions m_resourceLoaderOptions;
     BlockingBehavior m_blockingBehavior;
 
-    // |*m_peer| lives in the main thread.
-    CrossThreadPersistent<Peer> m_peer;
+    // |*m_mainThreadLoaderHolder| lives in the main thread.
+    CrossThreadPersistent<MainThreadLoaderHolder> m_mainThreadLoaderHolder;
 };
 
 } // namespace blink
 
 #endif // WorkerThreadableLoader_h