2010-06-22 Tony Gentilcore <tonyg@chromium.org>
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jun 2010 23:56:22 +0000 (23:56 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Jun 2010 23:56:22 +0000 (23:56 +0000)
        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):

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/fast/js/resources/script-line-number.js [new file with mode: 0644]
LayoutTests/fast/js/script-line-number-expected.txt [new file with mode: 0644]
LayoutTests/fast/js/script-line-number.html [new file with mode: 0644]
WebCore/ChangeLog
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/bindings/js/ScriptController.cpp
WebCore/bindings/js/ScriptController.h
WebCore/bindings/v8/ScriptController.cpp
WebCore/bindings/v8/ScriptController.h
WebCore/bindings/v8/ScriptEventListener.cpp
WebCore/bindings/v8/V8Proxy.h
WebCore/dom/XMLDocumentParserLibxml2.cpp
WebCore/html/HTML5DocumentParser.cpp
WebCore/html/HTML5ScriptRunner.h
WebCore/html/HTMLDocumentParser.cpp

index 3b424bb13f6a0253cd80be2a6db475420338d6ec..5986c84fb6e895fd380a20db563f5b3d2628046c 100644 (file)
@@ -1,3 +1,17 @@
+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>
 
         Reviewed by Adam Barth.
diff --git a/LayoutTests/fast/js/resources/script-line-number.js b/LayoutTests/fast/js/resources/script-line-number.js
new file mode 100644 (file)
index 0000000..09d9b1a
--- /dev/null
@@ -0,0 +1,26 @@
+description(
+"This test checks that line numbers are correctly reported for both inline scripts and inline event handlers."
+);
+
+function getLineFromError(e)
+{
+    // JSC
+    if (e.line)
+        return e.line;
+
+    // V8
+    if (e.stack) {
+        // ErrorName: ErrorDescription at FileName:LineNumber:ColumnNumber
+        parts = e.stack.split(":");
+        return parts[parts.length - 2];
+    }
+
+    return -1;
+}
+
+function assertErrorOnLine(error, expectedLine)
+{
+    shouldBe(stringify(getLineFromError(error)), stringify(expectedLine));
+}
+
+var successfullyParsed = true;
diff --git a/LayoutTests/fast/js/script-line-number-expected.txt b/LayoutTests/fast/js/script-line-number-expected.txt
new file mode 100644 (file)
index 0000000..f1db5f5
--- /dev/null
@@ -0,0 +1,11 @@
+This test checks that line numbers are correctly reported for both inline scripts and inline event handlers.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS 10 is 10
+PASS 13 is 13
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/js/script-line-number.html b/LayoutTests/fast/js/script-line-number.html
new file mode 100644 (file)
index 0000000..3c565e0
--- /dev/null
@@ -0,0 +1,20 @@
+<!DOCTYPE HTML PUBLIC "-//IETF//DTD HTML//EN">
+<html>
+<head>
+<link rel="stylesheet" href="resources/js-test-style.css">
+<script src="resources/js-test-pre.js"></script>
+</head>
+<body>
+<p id="description"></p>
+<div id="console"></div>
+<script src="resources/script-line-number.js" onload="try { nonExistentFunctionOnLine10(); } catch (e) { assertErrorOnLine(e, 10); }"></script>
+<script>
+try {
+    nonExistentFunctionOnLine13();
+} catch (e) {
+    assertErrorOnLine(e, 13);
+}
+</script>
+<script src="resources/js-test-post.js"></script>
+</body>
+</html>
index 774e626384fe101015e3c5818caed0f0c522bcb3..92a439ecce50b2f71cdd09e38de536e6868185a0 100644 (file)
@@ -1,3 +1,32 @@
+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>
 
         Reviewed by Eric Seidel.
index 539b0dd6a59332834a8578cb54aa29fb55b4373d..fc8a837c56f18202ad23c3bfb87834729eb2f9a7 100644 (file)
                                B56EBA8511C9FF8100B04477 /* SQLException.h in Headers */,
                                B525A96511CA2340003A23A8 /* JSSQLException.h in Headers */,
                                9719AF0011D09F2C00D45831 /* HTMLInputStream.h in Headers */,
-                               A853123D11D0471B00D4D077 /* FragmentScriptingPermission.h in Headers */,
+                               A853123D11D0471B00D4D077 /* FragmentScriptingPermission.h in Headers */,
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                };
index 5c4bd4c27f4853e69d7f234d2cca286f6b22f52a..6f0457a037f216bcd49ded443f919c0783e8a64e 100644 (file)
@@ -21,6 +21,7 @@
 #include "config.h"
 #include "ScriptController.h"
 
+#include "DocumentParser.h"
 #include "Event.h"
 #include "EventNames.h"
 #include "Frame.h"
@@ -60,7 +61,6 @@ void ScriptController::initializeThreading()
 
 ScriptController::ScriptController(Frame* frame)
     : m_frame(frame)
-    , m_handlerLineNumber(0)
     , m_sourceURL(0)
     , m_inExecuteScript(false)
     , m_processingTimerCallback(false)
@@ -235,6 +235,14 @@ JSDOMWindowShell* ScriptController::initScript(DOMWrapperWorld* world)
     return windowShell;
 }
 
+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
 {
     if (m_allowPopupsFromPlugin || isJavaScriptAnchorNavigation())
index ee7cc7d855a7ec8b496052c0111855f3bc23996a..0debf39b5dfd87d247db57ad42f0caed38c430a9 100644 (file)
@@ -117,9 +117,8 @@ public:
     ScriptValue evaluate(const ScriptSourceCode&, ShouldAllowXSS shouldAllowXSS = DoNotAllowXSS);
     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;
     bool anyPageIsProcessingUserGesture() const;
@@ -187,7 +186,6 @@ private:
 
     ShellMap m_windowShells;
     Frame* m_frame;
-    int m_handlerLineNumber;
     const String* m_sourceURL;
 
     bool m_inExecuteScript;
index 681e1a4b77e2052fdaf53cd30103ef593747d836..9f6c344ecdced6625d4441864884e3b1297b6a5a 100644 (file)
@@ -34,6 +34,7 @@
 
 #include "PlatformBridge.h"
 #include "Document.h"
+#include "DocumentParser.h"
 #include "DOMWindow.h"
 #include "Event.h"
 #include "EventListener.h"
@@ -258,9 +259,18 @@ ScriptValue ScriptController::evaluate(const ScriptSourceCode& sourceCode, Shoul
     return ScriptValue(object);
 }
 
-void ScriptController::setEventHandlerLineNumber(int lineNumber)
+int ScriptController::eventHandlerLineNumber() const
 {
-    m_proxy->setEventHandlerLineNumber(lineNumber);
+    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;
 }
 
 void ScriptController::finishedWithEvent(Event* event)
index 5d4b83e0dbb1c84aed875c973f6b8eaff113eb95..3187180b06adbcbace4c9e89b34e981bcc09008d 100644 (file)
@@ -153,7 +153,9 @@ public:
     static void gcUnprotectJSWrapper(void*);
 
     void finishedWithEvent(Event*);
-    void setEventHandlerLineNumber(int lineNumber);
+
+    int eventHandlerLineNumber() const;
+    int eventHandlerColumnNumber() const;
 
     void setProcessingTimerCallback(bool processingTimerCallback) { m_processingTimerCallback = processingTimerCallback; }
     // FIXME: Currently we don't use the parameter world at all.
index 57a2824a852658a96ecd032e451545866fd135cc..63e7dffa10977df4361fe45c23696a764843369b 100644 (file)
@@ -64,11 +64,8 @@ PassRefPtr<V8LazyEventListener> createAttributeEventListener(Node* node, Attribu
             return 0;
         }
 
-        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();
     }
 
@@ -97,11 +94,8 @@ PassRefPtr<V8LazyEventListener> createAttributeEventListener(Frame* frame, Attri
         return 0;
     }
 
-    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));
 }
index 088b33774bc6dedd7d1ae6af0b9f7557e7d42ae3..b16bd08c768a0536db67029fea524f3bc571046c 100644 (file)
@@ -202,7 +202,6 @@ namespace WebCore {
         }
 #endif
 
-        void setEventHandlerLineNumber(int lineNumber) { m_handlerLineNumber = lineNumber; }
         void finishedWithEvent(Event*) { }
 
         // Evaluate JavaScript in a new isolated world. The script gets its own
@@ -369,8 +368,6 @@ namespace WebCore {
         // For the moment, we have one of these.  Soon we will have one per DOMWrapperWorld.
         RefPtr<V8DOMWindowShell> m_windowShell;
 
-        int m_handlerLineNumber;
-
         // True for <a href="javascript:foo()"> and false for <script>foo()</script>.
         // Only valid during execution.
         bool m_inlineCode;
index ea17bfab1a316375ed1ee6c36f2bcf1d0195b9b0..01cfe49ac7cd9da4fb84fb08425bc4d81fe20d5d 100644 (file)
@@ -44,7 +44,6 @@
 #include "ResourceHandle.h"
 #include "ResourceRequest.h"
 #include "ResourceResponse.h"
-#include "ScriptController.h"
 #include "ScriptElement.h"
 #include "ScriptSourceCode.h"
 #include "ScriptValue.h"
@@ -783,19 +782,12 @@ void XMLDocumentParser::startElementNs(const xmlChar* xmlLocalName, const xmlCha
         return;
     }
 
-    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) {
         stopParsing();
         return;
     }
 
-    if (jsProxy)
-        jsProxy->setEventHandlerLineNumber(0);
-
     newElement->beginParsingChildren();
 
     ScriptElement* scriptElement = toScriptElement(newElement.get());
index 73a8db7ab96ab6b46810c4dd9c497a83c005eaa8..fe44afc480d6286d3b2fdd4e401e92e9b406fd5d 100644 (file)
@@ -220,16 +220,9 @@ void HTML5DocumentParser::pumpLexer(SynchronousMode mode)
         if (!m_lexer->nextToken(m_input.current(), m_token))
             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.
         if (!m_treeConstructor->isPaused())
             continue;
index 27311015bcda24057733e664a5fc66024b5035d4..9c2fb3602f4751c66b58a19c2601ab646b214504 100644 (file)
@@ -64,8 +64,8 @@ private:
     class PendingScript : public CachedResourceClient {
     public:
         PendingScript()
-            : m_watchingForLoad(false)
-            , startingLineNumber(0)
+            : startingLineNumber(0)
+            , m_watchingForLoad(false)
         {
         }
 
index 4bc467adce7d8e6483974e6b504971155e3713b4..70411f573abb46d3f9e320002fae1494f21f7255 100644 (file)
@@ -48,7 +48,6 @@
 #include "InspectorTimelineAgent.h"
 #include "Page.h"
 #include "PreloadScanner.h"
-#include "ScriptController.h"
 #include "ScriptSourceCode.h"
 #include "ScriptValue.h"
 #include "XSSAuditor.h"
@@ -1893,18 +1892,12 @@ bool HTMLDocumentParser::finishWasCalled()
 
 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);
         if (m_currentToken.tagName != commentAtom)
             m_currentToken.tagName = textAtom;
     } else if (m_currentToken.tagName == nullAtom) {
         m_currentToken.reset();
-        if (scriptController)
-            scriptController->setEventHandlerLineNumber(m_lineNumber + 1); // Script line numbers are 1 based.
         return 0;
     }
 
@@ -1922,8 +1915,6 @@ PassRefPtr<Node> HTMLDocumentParser::processToken()
             n = m_treeConstructor->parseToken(&m_currentToken);
     }
     m_currentToken.reset();
-    if (scriptController)
-        scriptController->setEventHandlerLineNumber(0);
 
     return n.release();
 }