Oilpan: have PendingScripts trace their script elements.

Source/core/dom/ScriptRunner.h

 /*
  * Copyright (C) 2010 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 10 matching lines @@
  * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
  * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
 #ifndef ScriptRunner_h
 #define ScriptRunner_h
 
 #include "core/fetch/ResourcePtr.h"
 #include "platform/Timer.h"
+#include "platform/heap/Handle.h"
 #include "wtf/HashMap.h"
 #include "wtf/Noncopyable.h"
 #include "wtf/PassOwnPtr.h"
 #include "wtf/Vector.h"
 
 namespace WebCore {
 
 class ScriptResource;
 class Document;
 class PendingScript;
 class ScriptLoader;
 
-class ScriptRunner {
-    WTF_MAKE_NONCOPYABLE(ScriptRunner); WTF_MAKE_FAST_ALLOCATED;
+class ScriptRunner FINAL : public NoBaseWillBeGarbageCollectedFinalized<ScriptRunner> {
+    WTF_MAKE_NONCOPYABLE(ScriptRunner); WTF_MAKE_FAST_ALLOCATED_WILL_BE_REMOVED;
 public:
-    static PassOwnPtr<ScriptRunner> create(Document* document) { return adoptPtr(new ScriptRunner(document)); }
+    static PassOwnPtrWillBeRawPtr<ScriptRunner> create(Document* document)
+    {
+        return adoptPtrWillBeNoop(new ScriptRunner(document));
+    }
     ~ScriptRunner();
 
     enum ExecutionType { ASYNC_EXECUTION, IN_ORDER_EXECUTION };
     void queueScriptForExecution(ScriptLoader*, ResourcePtr<ScriptResource>, ExecutionType);
     bool hasPendingScripts() const { return !m_scriptsToExecuteSoon.isEmpty() || !m_scriptsToExecuteInOrder.isEmpty() || !m_pendingAsyncScripts.isEmpty(); }
     void suspend();
     void resume();
     void notifyScriptReady(ScriptLoader*, ExecutionType);
     void notifyScriptLoadError(ScriptLoader*, ExecutionType);
 
+    void trace(Visitor*);
+
 private:
     explicit ScriptRunner(Document*);
 
     void timerFired(Timer<ScriptRunner>*);
 
-    Document* m_document;
+    RawPtrWillBeMember<Document> m_document;
+    // FIXME: Oilpan: consider using heap vectors and hash map here;
+    // PendingScript does have a (trivial) destructor, however.

[haraken, 2014/05/29 01:46:08]

Just help me understand: What prevents us from making these collections on-heap
in this CL?

[sof, 2014/05/29 06:02:07]

This goes to the core of this CL. If they move to the heap, we have a
finalization problem for PendingScript, as it cannot access its runner object
which is the script resource's ResourceClient. The runner object also being on
the heap.

[haraken, 2014/05/29 11:32:08]

Just help me understand: Where does ~PendingScript touch ResourceClient? I
failed to find the code...

[sof, 2014/05/29 20:39:54]

If you look at ResourceOwner<>::setResource() (PendingScript derives from
ResourceOwner), you may find it there.

Part of detaching/deleting a ResourceClient is to remove itself as a client from
the resources it has added itself to. If ~PendingScript were on the heap and
HTMLScriptRunner also, the finalization problem would manifest itself from
~PendingScript. That's what I referred to in the above/previous comment.

Does that help explaining?

[haraken, 2014/05/30 01:28:50]

I understand your point, but my question is:

If we move PendingScript to the heap and have the PendingScript and the
ResourceClient hold strong members to each other, we can make the PendingScript
and the ResourceClient die together. Then ResourceClient/PendingScript no longer
needs to touch PendingScript/ResourceClient in its destructor and thus the
touch-other-on-heap-objects-in-destructor issue is gone, isn't it?

[sof, 2014/05/30 06:26:40]

I just don't see how to correctly address the ResourceClient removing itself as
the client of the PendingScript's resource during finalization with that
arrangement.

They die together now and resources can be safely unregistered then by virtue of
the PendingScripts being part (or inline) objects of the script runner objects
(which are on the heap.)

[haraken, 2014/05/30 06:49:42]

Understood. I'm just wondering if we can eliminate the need of unregistering the
client by moving both client and PendingScript to the heap (then the
finalization issue will be gone because the client and the PendingScript die
together and thus we don't need to do the unregistration when the client dies).

Either way, I'm completely fine with the current CL. Mads and Eric are fine, so
I think you can go ahead and land. Sorry about interrupting you a lot.

[sof, 2014/05/30 09:43:40]

The problem is that PendingScript contains a Resource that will have to be
notified when the client dies. Unless the Resource client registration mechanism
is rethought/reconsidered (or Resource moves to the heap), then I just don't see
how this can work.
It makes for a better outcome, so I don't mind one bit really. Thanks :)

     Vector<PendingScript> m_scriptsToExecuteInOrder;
     Vector<PendingScript> m_scriptsToExecuteSoon; // http://www.whatwg.org/specs/web-apps/current-work/#set-of-scripts-that-will-execute-as-soon-as-possible
     HashMap<ScriptLoader*, PendingScript> m_pendingAsyncScripts;

[haraken, 2014/05/29 01:46:08]

The raw pointers in the key looks safe, since they are explicitly cleared in
notifyScriptReady/notifyScriptLoadError.

     Timer<ScriptRunner> m_timer;
 };
 
 }
 
 #endif