CTTE: ScriptElement always has a HTTPScriptElement or SVGScriptElement.

<https://webkit.org/b/134637>

Have ScriptElement store the pointer to the more specific subclass
element as a reference. Also made the constructor protected since
nobody should instantiate ScriptElement directly.

Reviewed by Antti Koivisto.

* dom/ScriptElement.cpp:
(WebCore::ScriptElement::ScriptElement):
(WebCore::ScriptElement::childrenChanged):
(WebCore::ScriptElement::dispatchErrorEvent):
(WebCore::ScriptElement::prepareScript):
(WebCore::ScriptElement::requestScript):
(WebCore::ScriptElement::executeScript):
(WebCore::ScriptElement::notifyFinished):
(WebCore::ScriptElement::ignoresLoadRequest):
(WebCore::ScriptElement::scriptContent):
* dom/ScriptElement.h:
(WebCore::ScriptElement::element):
* dom/ScriptRunner.cpp:
(WebCore::ScriptRunner::queueScriptForExecution):
* html/HTMLScriptElement.cpp:
(WebCore::HTMLScriptElement::HTMLScriptElement):
* svg/SVGScriptElement.cpp:
(WebCore::SVGScriptElement::SVGScriptElement):

Canonical link: https://commits.webkit.org/152613@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@170809 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,33 @@
+2014-07-04  Andreas Kling  <akling@apple.com>
+
+        CTTE: ScriptElement always has a HTTPScriptElement or SVGScriptElement.
+        <https://webkit.org/b/134637>
+
+        Have ScriptElement store the pointer to the more specific subclass
+        element as a reference. Also made the constructor protected since
+        nobody should instantiate ScriptElement directly.
+
+        Reviewed by Antti Koivisto.
+
+        * dom/ScriptElement.cpp:
+        (WebCore::ScriptElement::ScriptElement):
+        (WebCore::ScriptElement::childrenChanged):
+        (WebCore::ScriptElement::dispatchErrorEvent):
+        (WebCore::ScriptElement::prepareScript):
+        (WebCore::ScriptElement::requestScript):
+        (WebCore::ScriptElement::executeScript):
+        (WebCore::ScriptElement::notifyFinished):
+        (WebCore::ScriptElement::ignoresLoadRequest):
+        (WebCore::ScriptElement::scriptContent):
+        * dom/ScriptElement.h:
+        (WebCore::ScriptElement::element):
+        * dom/ScriptRunner.cpp:
+        (WebCore::ScriptRunner::queueScriptForExecution):
+        * html/HTMLScriptElement.cpp:
+        (WebCore::HTMLScriptElement::HTMLScriptElement):
+        * svg/SVGScriptElement.cpp:
+        (WebCore::SVGScriptElement::SVGScriptElement):
+
 2014-07-04  Julien Quint  <pom@graougraou.com>
 
         input type=range element should only fire change events after committing a  value

Source/WebCore/dom/ScriptElement.cpp

@@ -55,7 +55,7 @@
 
 namespace WebCore {
 
-ScriptElement::ScriptElement(Element* element, bool parserInserted, bool alreadyStarted)
+ScriptElement::ScriptElement(Element& element, bool parserInserted, bool alreadyStarted)
     : m_element(element)
     , m_cachedScript(0)
     , m_startLineNumber(WTF::OrdinalNumber::beforeFirst())
@@ -70,9 +70,8 @@ ScriptElement::ScriptElement(Element* element, bool parserInserted, bool already
     , m_willExecuteInOrder(false)
     , m_requestUsesAccessControl(false)
 {
-    ASSERT(m_element);
-    if (parserInserted && m_element->document().scriptableDocumentParser() && !m_element->document().isInDocumentWrite())
-        m_startLineNumber = m_element->document().scriptableDocumentParser()->textPosition().m_line;
+    if (parserInserted && m_element.document().scriptableDocumentParser() && !m_element.document().isInDocumentWrite())
+        m_startLineNumber = m_element.document().scriptableDocumentParser()->textPosition().m_line;
 }
 
 ScriptElement::~ScriptElement()
@@ -88,7 +87,7 @@ void ScriptElement::insertedInto(ContainerNode& insertionPoint)
 
 void ScriptElement::childrenChanged()
 {
-    if (!m_parserInserted && m_element->inDocument())
+    if (!m_parserInserted && m_element.inDocument())
         prepareScript(); // FIXME: Provide a real starting line number here.
 }
 
@@ -138,7 +137,7 @@ static bool isLegacySupportedJavaScriptLanguage(const String& language)
 
 void ScriptElement::dispatchErrorEvent()
 {
-    m_element->dispatchEvent(Event::create(eventNames().errorEvent, false, false));
+    m_element.dispatchEvent(Event::create(eventNames().errorEvent, false, false));
 }
 
 bool ScriptElement::isScriptTypeSupported(LegacyTypeSupport supportLegacyTypes) const
@@ -177,10 +176,10 @@ bool ScriptElement::prepareScript(const TextPosition& scriptStartPosition, Legac
         m_forceAsync = true;
 
     // FIXME: HTML5 spec says we should check that all children are either comments or empty text nodes.
-    if (!hasSourceAttribute() && !m_element->firstChild())
+    if (!hasSourceAttribute() && !m_element.firstChild())
         return false;
 
-    if (!m_element->inDocument())
+    if (!m_element.inDocument())
         return false;
 
     if (!isScriptTypeSupported(supportLegacyTypes))
@@ -194,7 +193,7 @@ bool ScriptElement::prepareScript(const TextPosition& scriptStartPosition, Legac
     m_alreadyStarted = true;
 
     // FIXME: If script is parser inserted, verify it's still in the original document.
-    Document& document = m_element->document();
+    Document& document = m_element.document();
 
     // FIXME: Eventually we'd like to evaluate scripts which are inserted into a
     // viewless document but this'll do for now.
@@ -230,7 +229,7 @@ bool ScriptElement::prepareScript(const TextPosition& scriptStartPosition, Legac
         document.scriptRunner()->queueScriptForExecution(this, m_cachedScript, ScriptRunner::IN_ORDER_EXECUTION);
         m_cachedScript->addClient(this);
     } else if (hasSourceAttribute()) {
-        m_element->document().scriptRunner()->queueScriptForExecution(this, m_cachedScript, ScriptRunner::ASYNC_EXECUTION);
+        m_element.document().scriptRunner()->queueScriptForExecution(this, m_cachedScript, ScriptRunner::ASYNC_EXECUTION);
         m_cachedScript->addClient(this);
     } else {
         // Reset line numbering for nested writes.
@@ -243,28 +242,28 @@ bool ScriptElement::prepareScript(const TextPosition& scriptStartPosition, Legac
 
 bool ScriptElement::requestScript(const String& sourceUrl)
 {
-    Ref<Document> originalDocument(m_element->document());
-    if (!m_element->dispatchBeforeLoadEvent(sourceUrl))
+    Ref<Document> originalDocument(m_element.document());
+    if (!m_element.dispatchBeforeLoadEvent(sourceUrl))
         return false;
-    if (!m_element->inDocument() || &m_element->document() != &originalDocument.get())
+    if (!m_element.inDocument() || &m_element.document() != &originalDocument.get())
         return false;
-    if (!m_element->document().contentSecurityPolicy()->allowScriptNonce(m_element->fastGetAttribute(HTMLNames::nonceAttr), m_element->document().url(), m_startLineNumber, m_element->document().completeURL(sourceUrl)))
+    if (!m_element.document().contentSecurityPolicy()->allowScriptNonce(m_element.fastGetAttribute(HTMLNames::nonceAttr), m_element.document().url(), m_startLineNumber, m_element.document().completeURL(sourceUrl)))
         return false;
 
     ASSERT(!m_cachedScript);
     if (!stripLeadingAndTrailingHTMLSpaces(sourceUrl).isEmpty()) {
-        CachedResourceRequest request(ResourceRequest(m_element->document().completeURL(sourceUrl)));
+        CachedResourceRequest request(ResourceRequest(m_element.document().completeURL(sourceUrl)));
 
-        String crossOriginMode = m_element->fastGetAttribute(HTMLNames::crossoriginAttr);
+        String crossOriginMode = m_element.fastGetAttribute(HTMLNames::crossoriginAttr);
         if (!crossOriginMode.isNull()) {
             m_requestUsesAccessControl = true;
             StoredCredentials allowCredentials = equalIgnoringCase(crossOriginMode, "use-credentials") ? AllowStoredCredentials : DoNotAllowStoredCredentials;
-            updateRequestForAccessControl(request.mutableResourceRequest(), m_element->document().securityOrigin(), allowCredentials);
+            updateRequestForAccessControl(request.mutableResourceRequest(), m_element.document().securityOrigin(), allowCredentials);
         }
         request.setCharset(scriptCharset());
-        request.setInitiator(element());
+        request.setInitiator(&element());
 
-        m_cachedScript = m_element->document().cachedResourceLoader()->requestScript(request);
+        m_cachedScript = m_element.document().cachedResourceLoader()->requestScript(request);
         m_isExternalScript = true;
     }
 
@@ -283,23 +282,23 @@ void ScriptElement::executeScript(const ScriptSourceCode& sourceCode)
     if (sourceCode.isEmpty())
         return;
 
-    if (!m_element->document().contentSecurityPolicy()->allowScriptNonce(m_element->fastGetAttribute(HTMLNames::nonceAttr), m_element->document().url(), m_startLineNumber))
+    if (!m_element.document().contentSecurityPolicy()->allowScriptNonce(m_element.fastGetAttribute(HTMLNames::nonceAttr), m_element.document().url(), m_startLineNumber))
         return;
 
-    if (!m_isExternalScript && !m_element->document().contentSecurityPolicy()->allowInlineScript(m_element->document().url(), m_startLineNumber))
+    if (!m_isExternalScript && !m_element.document().contentSecurityPolicy()->allowInlineScript(m_element.document().url(), m_startLineNumber))
         return;
 
 #if ENABLE(NOSNIFF)
     if (m_isExternalScript && m_cachedScript && !m_cachedScript->mimeTypeAllowedByNosniff()) {
-        m_element->document().addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Refused to execute script from '" + m_cachedScript->url().stringCenterEllipsizedToLength() + "' because its MIME type ('" + m_cachedScript->mimeType() + "') is not executable, and strict MIME type checking is enabled.");
+        m_element.document().addConsoleMessage(MessageSource::Security, MessageLevel::Error, "Refused to execute script from '" + m_cachedScript->url().stringCenterEllipsizedToLength() + "' because its MIME type ('" + m_cachedScript->mimeType() + "') is not executable, and strict MIME type checking is enabled.");
         return;
     }
 #endif
 
-    Ref<Document> document(m_element->document());
+    Ref<Document> document(m_element.document());
     if (Frame* frame = document->frame()) {
         IgnoreDestructiveWriteCountIncrementer ignoreDesctructiveWriteCountIncrementer(m_isExternalScript ? &document.get() : 0);
-        CurrentScriptIncrementer currentScriptIncrementer(&document.get(), m_element);
+        CurrentScriptIncrementer currentScriptIncrementer(&document.get(), &m_element);
 
         // Create a script from the script element node, using the script
         // block's source and the script block's type.
@@ -343,26 +342,26 @@ void ScriptElement::notifyFinished(CachedResource* resource)
         return;
 
     if (m_requestUsesAccessControl
-        && !m_element->document().securityOrigin()->canRequest(m_cachedScript->response().url())
-        && !m_cachedScript->passesAccessControlCheck(m_element->document().securityOrigin())) {
+        && !m_element.document().securityOrigin()->canRequest(m_cachedScript->response().url())
+        && !m_cachedScript->passesAccessControlCheck(m_element.document().securityOrigin())) {
 
         dispatchErrorEvent();
         DEPRECATED_DEFINE_STATIC_LOCAL(String, consoleMessage, (ASCIILiteral("Cross-origin script load denied by Cross-Origin Resource Sharing policy.")));
-        m_element->document().addConsoleMessage(MessageSource::JS, MessageLevel::Error, consoleMessage);
+        m_element.document().addConsoleMessage(MessageSource::JS, MessageLevel::Error, consoleMessage);
         return;
     }
 
     if (m_willExecuteInOrder)
-        m_element->document().scriptRunner()->notifyScriptReady(this, ScriptRunner::IN_ORDER_EXECUTION);
+        m_element.document().scriptRunner()->notifyScriptReady(this, ScriptRunner::IN_ORDER_EXECUTION);
     else
-        m_element->document().scriptRunner()->notifyScriptReady(this, ScriptRunner::ASYNC_EXECUTION);
+        m_element.document().scriptRunner()->notifyScriptReady(this, ScriptRunner::ASYNC_EXECUTION);
 
     m_cachedScript = 0;
 }
 
 bool ScriptElement::ignoresLoadRequest() const
 {
-    return m_alreadyStarted || m_isExternalScript || m_parserInserted || !m_element->inDocument();
+    return m_alreadyStarted || m_isExternalScript || m_parserInserted || !m_element.inDocument();
 }
 
 bool ScriptElement::isScriptForEventSupported() const
@@ -383,7 +382,7 @@ bool ScriptElement::isScriptForEventSupported() const
 
 String ScriptElement::scriptContent() const
 {
-    return TextNodeTraversal::contentsAsString(m_element);
+    return TextNodeTraversal::contentsAsString(&m_element);
 }
 
 ScriptElement* toScriptElementIfPossible(Element* element)

Source/WebCore/dom/ScriptElement.h

@@ -36,10 +36,10 @@ class ScriptSourceCode;
 
 class ScriptElement : private CachedResourceClient {
 public:
-    ScriptElement(Element*, bool createdByParser, bool isEvaluated);
     virtual ~ScriptElement();
 
-    Element* element() const { return m_element; }
+    Element& element() { return m_element; }
+    const Element& element() const { return m_element; }
 
     enum LegacyTypeSupport { DisallowLegacyTypeInTypeAttribute, AllowLegacyTypeInTypeAttribute };
     bool prepareScript(const TextPosition& scriptStartPosition = TextPosition::minimumPosition(), LegacyTypeSupport = DisallowLegacyTypeInTypeAttribute);
@@ -61,6 +61,8 @@ class ScriptElement : private CachedResourceClient {
     CachedResourceHandle<CachedScript> cachedScript() { return m_cachedScript; }
 
 protected:
+    ScriptElement(Element&, bool createdByParser, bool isEvaluated);
+
     void setHaveFiredLoadEvent(bool haveFiredLoad) { m_haveFiredLoad = haveFiredLoad; }
     bool isParserInserted() const { return m_parserInserted; }
     bool alreadyStarted() const { return m_alreadyStarted; }
@@ -91,7 +93,7 @@ class ScriptElement : private CachedResourceClient {
     virtual bool deferAttributeValue() const = 0;
     virtual bool hasSourceAttribute() const = 0;
 
-    Element* m_element;
+    Element& m_element;
     CachedResourceHandle<CachedScript> m_cachedScript;
     WTF::OrdinalNumber m_startLineNumber;
     bool m_parserInserted : 1;

Source/WebCore/dom/ScriptRunner.cpp

@@ -54,19 +54,18 @@ void ScriptRunner::queueScriptForExecution(ScriptElement* scriptElement, CachedR
     ASSERT(scriptElement);
     ASSERT(cachedScript.get());
 
-    Element* element = scriptElement->element();
-    ASSERT(element);
-    ASSERT(element->inDocument());
+    Element& element = scriptElement->element();
+    ASSERT(element.inDocument());
 
     m_document.incrementLoadEventDelayCount();
 
     switch (executionType) {
     case ASYNC_EXECUTION:
-        m_pendingAsyncScripts.add(scriptElement, PendingScript(element, cachedScript.get()));
+        m_pendingAsyncScripts.add(scriptElement, PendingScript(&element, cachedScript.get()));
         break;
 
     case IN_ORDER_EXECUTION:
-        m_scriptsToExecuteInOrder.append(PendingScript(element, cachedScript.get()));
+        m_scriptsToExecuteInOrder.append(PendingScript(&element, cachedScript.get()));
         break;
     }
 }

Source/WebCore/html/HTMLScriptElement.cpp

@@ -37,7 +37,7 @@ using namespace HTMLNames;
 
 inline HTMLScriptElement::HTMLScriptElement(const QualifiedName& tagName, Document& document, bool wasInsertedByParser, bool alreadyStarted)
     : HTMLElement(tagName, document)
-    , ScriptElement(this, wasInsertedByParser, alreadyStarted)
+    , ScriptElement(*this, wasInsertedByParser, alreadyStarted)
 {
     ASSERT(hasTagName(scriptTag));
 }

Source/WebCore/svg/SVGScriptElement.cpp

@@ -44,7 +44,7 @@ END_REGISTER_ANIMATED_PROPERTIES
 
 inline SVGScriptElement::SVGScriptElement(const QualifiedName& tagName, Document& document, bool wasInsertedByParser, bool alreadyStarted)
     : SVGElement(tagName, document)
-    , ScriptElement(this, wasInsertedByParser, alreadyStarted)
+    , ScriptElement(*this, wasInsertedByParser, alreadyStarted)
     , m_svgLoadEventTimer(this, &SVGElement::svgLoadEventTimerFired)
 {
     ASSERT(hasTagName(SVGNames::scriptTag));