Changeset 61640 in webkit

Timestamp:

Jun 22, 2010 4:56:22 PM (14 years ago)

Author:

abarth@webkit.org

Message:

2010-06-22 Tony Gentilcore <​tonyg@chromium.org>

Reviewed by Eric Seidel.

Add layout tests which test line numbers for both inline and event handler
scripts in HTML documents.
​https://bugs.webkit.org/show_bug.cgi?id=40649

• fast/js/resources/script-line-number.js: Added. (getLineFromError): (assertErrorOnLine):
• fast/js/script-line-number-expected.txt: Added.
• fast/js/script-line-number.html: Added.

2010-06-22 Tony Gentilcore <​tonyg@chromium.org>

Reviewed by Eric Seidel.

Pull script line number from DocumentParser instead of pushing it to ScriptController
​https://bugs.webkit.org/show_bug.cgi?id=40649

This approach is cleaner and improves WebCore/benchmarks/parser/html-parser.html by ~2%.

Tests: fast/js/script-line-number.html

• bindings/js/ScriptController.cpp: (WebCore::ScriptController::ScriptController): (WebCore::ScriptController::eventHandlerLineNumber):
• bindings/js/ScriptController.h:
• bindings/v8/ScriptController.cpp: (WebCore::ScriptController::eventHandlerLineNumber): (WebCore::ScriptController::eventHandlerColumnNumber):
• bindings/v8/ScriptController.h:
• bindings/v8/ScriptEventListener.cpp: (WebCore::createAttributeEventListener):
• bindings/v8/V8Proxy.h:
• dom/XMLDocumentParserLibxml2.cpp: (WebCore::XMLDocumentParser::startElementNs):
• html/HTML5DocumentParser.cpp: (WebCore::HTML5DocumentParser::pumpLexer):
• html/HTMLDocumentParser.cpp: (WebCore::HTMLDocumentParser::processToken):

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/js/resources/script-line-number.js (added)
• LayoutTests/fast/js/script-line-number-expected.txt (added)
• LayoutTests/fast/js/script-line-number.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/WebCore.xcodeproj/project.pbxproj (modified) (1 diff)
• WebCore/bindings/js/ScriptController.cpp (modified) (3 diffs)
• WebCore/bindings/js/ScriptController.h (modified) (2 diffs)
• WebCore/bindings/v8/ScriptController.cpp (modified) (2 diffs)
• WebCore/bindings/v8/ScriptController.h (modified) (1 diff)
• WebCore/bindings/v8/ScriptEventListener.cpp (modified) (2 diffs)
• WebCore/bindings/v8/V8Proxy.h (modified) (2 diffs)
• WebCore/dom/XMLDocumentParserLibxml2.cpp (modified) (3 diffs)
• WebCore/html/HTML5DocumentParser.cpp (modified) (1 diff)
• WebCore/html/HTML5ScriptRunner.h (modified) (1 diff)
• WebCore/html/HTMLDocumentParser.cpp (modified) (4 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2010-06-22  Tony Gentilcore  <tonyg@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        Add layout tests which test line numbers for both inline and event handler
+        scripts in HTML documents.
+        https://bugs.webkit.org/show_bug.cgi?id=40649
+
+        * fast/js/resources/script-line-number.js: Added.
+        (getLineFromError):
+        (assertErrorOnLine):
+        * fast/js/script-line-number-expected.txt: Added.
+        * fast/js/script-line-number.html: Added.
+
 2010-06-22  Eric Seidel  <eric@webkit.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-06-22  Tony Gentilcore  <tonyg@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        Pull script line number from DocumentParser instead of pushing it to ScriptController
+        https://bugs.webkit.org/show_bug.cgi?id=40649
+
+        This approach is cleaner and improves WebCore/benchmarks/parser/html-parser.html by ~2%.
+
+        Tests: fast/js/script-line-number.html
+
+        * bindings/js/ScriptController.cpp:
+        (WebCore::ScriptController::ScriptController):
+        (WebCore::ScriptController::eventHandlerLineNumber):
+        * bindings/js/ScriptController.h:
+        * bindings/v8/ScriptController.cpp:
+        (WebCore::ScriptController::eventHandlerLineNumber):
+        (WebCore::ScriptController::eventHandlerColumnNumber):
+        * bindings/v8/ScriptController.h:
+        * bindings/v8/ScriptEventListener.cpp:
+        (WebCore::createAttributeEventListener):
+        * bindings/v8/V8Proxy.h:
+        * dom/XMLDocumentParserLibxml2.cpp:
+        (WebCore::XMLDocumentParser::startElementNs):
+        * html/HTML5DocumentParser.cpp:
+        (WebCore::HTML5DocumentParser::pumpLexer):
+        * html/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::processToken):
+
 2010-06-22  Tony Gentilcore  <tonyg@chromium.org>
 

trunk/WebCore/WebCore.xcodeproj/project.pbxproj

@@ -19503 +19503 @@
                                 B525A96511CA2340003A23A8 /* JSSQLException.h in Headers */,
                                 9719AF0011D09F2C00D45831 /* HTMLInputStream.h in Headers */,
-                                A853123D11D0471B00D4D077 /* FragmentScriptingPermission.h in Headers */,
+                                A853123D11D0471B00D4D077 /* FragmentScriptingPermission.h in Headers */,
                         );
                         runOnlyForDeploymentPostprocessing = 0;

trunk/WebCore/bindings/js/ScriptController.cpp

@@ -22 +22 @@
 #include "ScriptController.h"
 
+#include "DocumentParser.h"
 #include "Event.h"
 #include "EventNames.h"
@@ -61 +62 @@
 ScriptController::ScriptController(Frame* frame)
     : m_frame(frame)
-    , m_handlerLineNumber(0)
     , m_sourceURL(0)
     , m_inExecuteScript(false)
@@ -236 +236 @@
 }
 
+int ScriptController::eventHandlerLineNumber() const
+{
+    // JSC expects 1-based line numbers, so we must add one here to get it right.
+    if (DocumentParser* parser = m_frame->document()->parser())
+        return parser->lineNumber() + 1;
+    return 0;
+}
+    
 bool ScriptController::processingUserGesture(DOMWrapperWorld* world) const
 {

trunk/WebCore/bindings/js/ScriptController.h

@@ -118 +118 @@
     ScriptValue evaluateInWorld(const ScriptSourceCode&, DOMWrapperWorld*, ShouldAllowXSS shouldAllowXSS = DoNotAllowXSS);
 
-    void setEventHandlerLineNumber(int lineno) { m_handlerLineNumber = lineno; }
-    int eventHandlerLineNumber() { return m_handlerLineNumber; }
-
+    int eventHandlerLineNumber() const;
+    
     void setProcessingTimerCallback(bool b) { m_processingTimerCallback = b; }
     bool processingUserGesture(DOMWrapperWorld*) const;
@@ -188 +187 @@
     ShellMap m_windowShells;
     Frame* m_frame;
-    int m_handlerLineNumber;
     const String* m_sourceURL;
 

trunk/WebCore/bindings/v8/ScriptController.cpp

@@ -35 +35 @@
 #include "PlatformBridge.h"
 #include "Document.h"
+#include "DocumentParser.h"
 #include "DOMWindow.h"
 #include "Event.h"
@@ -259 +260 @@
 }
 
-void ScriptController::setEventHandlerLineNumber(int lineNumber)
-{
-    m_proxy->setEventHandlerLineNumber(lineNumber);
+int ScriptController::eventHandlerLineNumber() const
+{
+    if (DocumentParser* parser = m_frame->document()->parser())
+        return parser->lineNumber();
+    return 0;
+}
+
+int ScriptController::eventHandlerColumnNumber() const
+{
+    if (DocumentParser* parser = m_frame->document()->parser())
+        return parser->columnNumber();
+    return 0;
 }
 

trunk/WebCore/bindings/v8/ScriptController.h

@@ -154 +154 @@
 
     void finishedWithEvent(Event*);
-    void setEventHandlerLineNumber(int lineNumber);
+
+    int eventHandlerLineNumber() const;
+    int eventHandlerColumnNumber() const;
 
     void setProcessingTimerCallback(bool processingTimerCallback) { m_processingTimerCallback = processingTimerCallback; }

trunk/WebCore/bindings/v8/ScriptEventListener.cpp

@@ -65 +65 @@
         }
 
-        if (frame->document()->parser()) {
-            // FIXME: Change to use script->eventHandlerLineNumber() when implemented.
-            lineNumber = frame->document()->parser()->lineNumber();
-            columnNumber = frame->document()->parser()->columnNumber();
-        }
+        lineNumber = scriptController->eventHandlerLineNumber();
+        columnNumber = scriptController->eventHandlerColumnNumber();
         sourceURL = node->document()->url().string();
     }
@@ -98 +95 @@
     }
 
-    if (frame->document()->parser()) {
-        // FIXME: Change to use script->eventHandlerLineNumber() when implemented.
-        lineNumber = frame->document()->parser()->lineNumber();
-        columnNumber = frame->document()->parser()->columnNumber();
-    }
+    lineNumber = scriptController->eventHandlerLineNumber();
+    columnNumber = scriptController->eventHandlerColumnNumber();
     sourceURL = frame->document()->url().string();
     return V8LazyEventListener::create(attr->localName().string(), frame->document()->isSVGDocument(), attr->value(), sourceURL, lineNumber, columnNumber, WorldContextHandle(UseMainWorld));

trunk/WebCore/bindings/v8/V8Proxy.h

@@ -203 +203 @@
 #endif
 
-        void setEventHandlerLineNumber(int lineNumber) { m_handlerLineNumber = lineNumber; }
         void finishedWithEvent(Event*) { }
 
@@ -370 +369 @@
         RefPtr<V8DOMWindowShell> m_windowShell;
 
-        int m_handlerLineNumber;
-
         // True for <a href="javascript:foo()"> and false for <script>foo()</script>.
         // Only valid during execution.

trunk/WebCore/dom/XMLDocumentParserLibxml2.cpp

@@ -45 +45 @@
 #include "ResourceRequest.h"
 #include "ResourceResponse.h"
-#include "ScriptController.h"
 #include "ScriptElement.h"
 #include "ScriptSourceCode.h"
@@ -784 +783 @@
     }
 
-    ScriptController* jsProxy = m_doc->frame() ? m_doc->frame()->script() : 0;
-    if (jsProxy && m_doc->frame()->script()->canExecuteScripts(NotAboutToExecuteScript))
-        jsProxy->setEventHandlerLineNumber(lineNumber());
-
     handleElementAttributes(newElement.get(), libxmlAttributes, nb_attributes, ec, m_scriptingPermission);
     if (ec) {
@@ -793 +788 @@
         return;
     }
-
-    if (jsProxy)
-        jsProxy->setEventHandlerLineNumber(0);
 
     newElement->beginParsingChildren();

trunk/WebCore/html/HTML5DocumentParser.cpp

@@ -221 +221 @@
             break;
 
-        if (ScriptController* scriptController = script())
-            scriptController->setEventHandlerLineNumber(lineNumber() + 1);
-
         m_treeConstructor->constructTreeFromToken(m_token);
         m_token.clear();
-
-        // FIXME: Why is setEventHandlerLineNumber(0) necessary?
-        if (ScriptController* scriptController = script())
-            scriptController->setEventHandlerLineNumber(0);
 
         // The parser will pause itself when waiting on a script to load or run.

trunk/WebCore/html/HTML5ScriptRunner.h

@@ -65 +65 @@
     public:
         PendingScript()
-            : m_watchingForLoad(false)
-            , startingLineNumber(0)
+            : startingLineNumber(0)
+            , m_watchingForLoad(false)
         {
         }

trunk/WebCore/html/HTMLDocumentParser.cpp

@@ -49 +49 @@
 #include "Page.h"
 #include "PreloadScanner.h"
-#include "ScriptController.h"
 #include "ScriptSourceCode.h"
 #include "ScriptValue.h"
@@ -1894 +1893 @@
 PassRefPtr<Node> HTMLDocumentParser::processToken()
 {
-    ScriptController* scriptController = (!m_fragment && m_doc->frame()) ? m_doc->frame()->script() : 0;
-    if (scriptController && scriptController->canExecuteScripts(NotAboutToExecuteScript))
-        // FIXME: Why isn't this m_currentScriptTagStartLineNumber?  I suspect this is wrong.
-        scriptController->setEventHandlerLineNumber(m_currentTagStartLineNumber + 1); // Script line numbers are 1 based.
     if (m_dest > m_buffer) {
         m_currentToken.text = StringImpl::createStrippingNullCharacters(m_buffer, m_dest - m_buffer);
@@ -1904 +1899 @@
     } else if (m_currentToken.tagName == nullAtom) {
         m_currentToken.reset();
-        if (scriptController)
-            scriptController->setEventHandlerLineNumber(m_lineNumber + 1); // Script line numbers are 1 based.
         return 0;
     }
@@ -1923 +1916 @@
     }
     m_currentToken.reset();
-    if (scriptController)
-        scriptController->setEventHandlerLineNumber(0);
 
     return n.release();