2011-01-04 Tony Gentilcore <tonyg@chromium.org>
authortonyg@chromium.org <tonyg@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jan 2011 05:30:02 +0000 (05:30 +0000)
committertonyg@chromium.org <tonyg@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 5 Jan 2011 05:30:02 +0000 (05:30 +0000)
        Reviewed by Ryosuke Niwa.

        Avoid manual ref/deref in AsyncScriptRunner by using PendingScript
        https://bugs.webkit.org/show_bug.cgi?id=51723

        ScriptElement should also be able to keep its Element+CachedScript in
        a PendingScript, and then executeScriptSoon can accept a PendingScript.
        Once HTMLScriptRunner, ScriptElement, and AsyncScriptRunner all use
        PendingScripts, then generic request and execute which operate on
        PendingScripts can be factored out to avoid code duplicate that we have.

        No new tests because no new functionality.

        * dom/AsyncScriptRunner.cpp:
        (WebCore::AsyncScriptRunner::~AsyncScriptRunner):
        (WebCore::AsyncScriptRunner::executeScriptSoon):
        (WebCore::AsyncScriptRunner::timerFired):
        * dom/AsyncScriptRunner.h:
        * dom/PendingScript.h: Add ctor which sets element and cachedScript. Rename adoptElement->setElement.
        * dom/HTMLScriptRunner.cpp:
        (WebCore::HTMLScriptRunner::requestPendingScript):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@75045 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/dom/AsyncScriptRunner.cpp
WebCore/dom/AsyncScriptRunner.h
WebCore/dom/PendingScript.h
WebCore/html/parser/HTMLScriptRunner.cpp

index 16a964b..0c3704d 100644 (file)
@@ -1,3 +1,27 @@
+2011-01-04  Tony Gentilcore  <tonyg@chromium.org>
+
+        Reviewed by Ryosuke Niwa.
+
+        Avoid manual ref/deref in AsyncScriptRunner by using PendingScript
+        https://bugs.webkit.org/show_bug.cgi?id=51723
+
+        ScriptElement should also be able to keep its Element+CachedScript in
+        a PendingScript, and then executeScriptSoon can accept a PendingScript.
+        Once HTMLScriptRunner, ScriptElement, and AsyncScriptRunner all use
+        PendingScripts, then generic request and execute which operate on
+        PendingScripts can be factored out to avoid code duplicate that we have.
+
+        No new tests because no new functionality.
+
+        * dom/AsyncScriptRunner.cpp:
+        (WebCore::AsyncScriptRunner::~AsyncScriptRunner):
+        (WebCore::AsyncScriptRunner::executeScriptSoon):
+        (WebCore::AsyncScriptRunner::timerFired):
+        * dom/AsyncScriptRunner.h:
+        * dom/PendingScript.h: Add ctor which sets element and cachedScript. Rename adoptElement->setElement.
+        * dom/HTMLScriptRunner.cpp:
+        (WebCore::HTMLScriptRunner::requestPendingScript):
+
 2011-01-04  Jeff Miller  <jeffm@apple.com>
 
         Reviewed by Darin Adler.
index 7f5f1b6..28b1b31 100644 (file)
@@ -29,6 +29,7 @@
 #include "CachedScript.h"
 #include "Document.h"
 #include "Element.h"
+#include "PendingScript.h"
 #include "ScriptElement.h"
 
 namespace WebCore {
@@ -42,23 +43,20 @@ AsyncScriptRunner::AsyncScriptRunner(Document* document)
 
 AsyncScriptRunner::~AsyncScriptRunner()
 {
-    for (size_t i = 0; i < m_scriptsToExecuteSoon.size(); ++i) {
-        m_scriptsToExecuteSoon[i].first->element()->deref(); // Balances ref() in executeScriptSoon().
+    for (size_t i = 0; i < m_scriptsToExecuteSoon.size(); ++i)
         m_document->decrementLoadEventDelayCount();
-    }
 }
 
-void AsyncScriptRunner::executeScriptSoon(ScriptElement* data, CachedResourceHandle<CachedScript> cachedScript)
+void AsyncScriptRunner::executeScriptSoon(ScriptElement* scriptElement, CachedResourceHandle<CachedScript> cachedScript)
 {
-    ASSERT_ARG(data, data);
+    ASSERT_ARG(scriptElement, scriptElement);
 
-    Element* element = data->element();
+    Element* element = scriptElement->element();
     ASSERT(element);
     ASSERT(element->inDocument());
 
     m_document->incrementLoadEventDelayCount();
-    m_scriptsToExecuteSoon.append(make_pair(data, cachedScript));
-    element->ref(); // Balanced by deref()s in timerFired() and dtor.
+    m_scriptsToExecuteSoon.append(PendingScript(element, cachedScript.get()));
     if (!m_timer.isActive())
         m_timer.startOneShot(0);
 }
@@ -80,12 +78,13 @@ void AsyncScriptRunner::timerFired(Timer<AsyncScriptRunner>* timer)
 
     RefPtr<Document> protect(m_document);
     
-    Vector<pair<ScriptElement*, CachedResourceHandle<CachedScript> > > scripts;
+    Vector<PendingScript> scripts;
     scripts.swap(m_scriptsToExecuteSoon);
     size_t size = scripts.size();
     for (size_t i = 0; i < size; ++i) {
-        scripts[i].first->execute(scripts[i].second.get());
-        scripts[i].first->element()->deref(); // Balances ref() in executeScriptSoon().
+        CachedScript* cachedScript = scripts[i].cachedScript();
+        RefPtr<Element> element = scripts[i].releaseElementAndClear();
+        toScriptElement(element.get())->execute(cachedScript);
         m_document->decrementLoadEventDelayCount();
     }
 }
index 1a051a8..2326f67 100644 (file)
@@ -36,6 +36,7 @@ namespace WebCore {
 
 class CachedScript;
 class Document;
+class PendingScript;
 class ScriptElement;
     
 class AsyncScriptRunner : public Noncopyable {
@@ -54,7 +55,7 @@ private:
     void timerFired(Timer<AsyncScriptRunner>*);
 
     Document* m_document;
-    Vector<std::pair<ScriptElement*, CachedResourceHandle<CachedScript> > > m_scriptsToExecuteSoon;
+    Vector<PendingScript> m_scriptsToExecuteSoon; // http://www.whatwg.org/specs/web-apps/current-work/#set-of-scripts-that-will-execute-as-soon-as-possible
     Timer<AsyncScriptRunner> m_timer;
 };
 
index 9c4c48b..fcbe06e 100644 (file)
@@ -49,6 +49,13 @@ public:
     {
     }
 
+    PendingScript(Element* element, CachedScript* cachedScript)
+        : m_watchingForLoad(false)
+        , m_element(element)
+    {
+        setCachedScript(cachedScript);
+    }
+
     PendingScript(const PendingScript& other)
         : CachedResourceClient(other)
         , m_watchingForLoad(other.m_watchingForLoad)
@@ -80,7 +87,7 @@ public:
     void setWatchingForLoad(bool b) { m_watchingForLoad = b; }
 
     Element* element() const { return m_element.get(); }
-    void adoptElement(Element* element) { m_element = element; }
+    void setElement(Element* element) { m_element = element; }
     PassRefPtr<Element> releaseElementAndClear();
 
     CachedScript* cachedScript() const;
index 5e70546..2fe1d30 100644 (file)
@@ -269,7 +269,7 @@ bool HTMLScriptRunner::requestPendingScript(PendingScript& pendingScript, Elemen
     // FIXME: We need to resolve the url relative to the element.
     if (!script->dispatchBeforeLoadEvent(srcValue))
         return false;
-    pendingScript.adoptElement(script);
+    pendingScript.setElement(script);
     // This should correctly return 0 for empty or invalid srcValues.
     CachedScript* cachedScript = m_document->cachedResourceLoader()->requestScript(srcValue, toScriptElement(script)->scriptCharset());
     if (!cachedScript) {
@@ -308,7 +308,7 @@ void HTMLScriptRunner::runScript(Element* script, const TextPosition1& scriptSta
             // The latter case can only happen if a script both triggers a stylesheet load
             // and writes an inline script. Since write is blocking we have to execute the
             // written script immediately, ignoring the pending sheets.
-            m_parsingBlockingScript.adoptElement(script);
+            m_parsingBlockingScript.setElement(script);
             m_parsingBlockingScript.setStartingPosition(scriptStartPosition);
         } else {
             ASSERT(isExecutingScript());