Oilpan: have PendingScripts trace their script elements.

Source/core/dom/PendingScript.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 11 matching lines @@
  * (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 PendingScript_h
 #define PendingScript_h
 
 #include "core/fetch/ResourceClient.h"
 #include "core/fetch/ResourceOwner.h"
 #include "core/fetch/ScriptResource.h"
+#include "platform/heap/Handle.h"
 #include "wtf/PassRefPtr.h"
 #include "wtf/RefPtr.h"
 #include "wtf/text/TextPosition.h"
 
 namespace WebCore {
 
 class Element;
 class ScriptResource;
 
 // A container for an external script which may be loaded and executed.
 //
 // A ResourcePtr alone does not prevent the underlying Resource
 // from purging its data buffer. This class holds a dummy client open for its
 // lifetime in order to guarantee that the data buffer will not be purged.
-class PendingScript : public ResourceOwner<ScriptResource> {
+class PendingScript FINAL : public ResourceOwner<ScriptResource> {
 public:
     PendingScript()
         : m_watchingForLoad(false)
         , m_startingPosition(TextPosition::belowRangePosition())
     {
     }
 
     PendingScript(Element* element, ScriptResource* resource)
         : m_watchingForLoad(false)
         , m_element(element)
@@ skipping 26 matching lines @@
     }
 
     TextPosition startingPosition() const { return m_startingPosition; }
     void setStartingPosition(const TextPosition& position) { m_startingPosition = position; }
 
     bool watchingForLoad() const { return m_watchingForLoad; }
     void setWatchingForLoad(bool b) { m_watchingForLoad = b; }
 
     Element* element() const { return m_element.get(); }
     void setElement(Element* element) { m_element = element; }
-    PassRefPtr<Element> releaseElementAndClear();
+    PassRefPtrWillBeRawPtr<Element> releaseElementAndClear();
 
     void setScriptResource(ScriptResource*);
 
     virtual void notifyFinished(Resource*);
 
+    void trace(Visitor*);
+
 private:
     bool m_watchingForLoad;
-    RefPtr<Element> m_element;
+    // FIXME: Oilpan: PendingScript must trace its script element, but
+    // turning the object into a GCed object is problematic. Due to
+    // external ownership arrangements: a PendingScript is
+    // conceptually owned by a script runner object, which when
+    // finalized will have to coordinate the release of the pending
+    // script resource with that of its owner/host
+    // (cf. HTMLScriptRunner finalization.) Doing that when host +
+    // owner + PendingScript are all heap allocated and independently
+    // finalized is not easily achieveable, hence we currently keep
+    // PendingScript off heap, along with its immediate owner.

[haraken, 2014/05/26 01:08:25]

Hmm, can we probably address the issue using weak processing?

By calling registerWeakMembers(this, weakCallback), the weak processing is
called back when the |this| object is going to die. In the weak processing,
you're allowed to touch other on-heap objects that might die in the same GC
cycle. (However, you're not allowed to allocate anything.)

[sof, 2014/05/26 09:41:34]

If HTMLScriptRunner and HTMLDocumentParser both die at the same time, then no
weak callback gets to run, right? (as that's dependent on the trace() of either
gets to run & register a weak callback.)

If so, then I don't see how this could work in this context.

[Mads Ager (chromium), 2014/05/26 13:55:36]

Yeah, I see the issue. For now I think we need to keep the OwnPtrs here to make
sure that these owned objects are basically part of the DocumentParser so it can
unregister itself from the pending resources when it dies.

I guess one way of dealing with this longer term would be to get Resource onto
our heap and make the set of clients a weak collection.

The current code is pretty nasty:

DocumentParser
    OwnPtr<HTMLScriptRunner>

~HTMLScriptRunner
    stopWatchingForLoad(...);

void HTMLScriptRunner::stopWatchingForLoad(PendingScript& pendingScript)
{
    ASSERT(pendingScript.watchingForLoad());
    m_host->stopWatchingForLoad(pendingScript.resource());
    pendingScript.setWatchingForLoad(false);
}

We are at the end of the destruction of m_host at this point, but let's call a
method on it! :-)

[sof, 2014/05/26 21:10:18]

That gave me an idea -- if HTMLScriptRunner is made the ResourceClient instead,
and it is able to handle the finalization without relying on the 'host object',
then we can simplify. The HTMLScriptRunner would then have to redirect the
notifications of finished resource loads to its host, but I think that's an
acceptable arrangement.

+    //
+    // Do want to trace() PendingScripts, so we do annotate the
+    // element as being a Member, but disable GC plugin checks for it.
+    GC_PLUGIN_IGNORE("357163")
+    RefPtrWillBeMember<Element> m_element;
     TextPosition m_startingPosition; // Only used for inline script tags.
 };
 
 }
 
 #endif