Bug 15195 – KJSProxy::m_handlerLineno is not reset between page loads in some cases

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Matt Perry]

Run this command (all one line):
run-webkit-tests dom/html/level1/core/documentinvalidcharacterexceptioncreateentref.html dom/xhtml/level2/html/frame.xhtml 


The second test will fail.  The frame.xhtml test has dependencies on previous test runs*.  The reason it doesn't fail normally is that run-webkit-tests restarts DumpRenderTree every 1000 tests, and during normal execution, frame.xhtml is run in a more "fresh" DumpRenderTree.  (Another way to see this failure is to bump up the testsPerDumpTool to a large value, say 10000).

* The specific dependency here is that KJSProxy::m_handlerLineno is not reset between pages in some cases.  This shows up in the DRT output as a CONSOLE MESSAGE with an invalid line number.

[Matt Perry]

Resummarizing to match the core bug here (it's probably not feasible to remove the interdependency of layout tests).

[David Kilzer (:ddkilzer)]

Confirmed with a local debug build of WebKit r25529 and DRT.

[Eric Seidel (no email)]

Another easy fix.

[Eric Seidel (no email)]

Created attachment 18431 [details]
Reset KJSProxy::m_handlerLineno in KJSProxy::clear

 WebCore/ChangeLog                 |   14 ++++++++++++++
 WebCore/bindings/js/kjs_proxy.cpp |    4 ++++
 2 files changed, 18 insertions(+), 0 deletions(-)

[Darin Adler]

Comment on attachment 18431 [details]
Reset KJSProxy::m_handlerLineno in KJSProxy::clear

Here are some things I notice:

    1) XML doesn't set the line number at all while parsing.

    2) The functions to create event handlers should ask the document's tokenizer what the current line number is, perhaps with a special function that is specifically designed for event handlers, rather than storing a line number inside the KJSProxy object, which seems upside down and backwards to me.

    3) Setting the line number back to 0 is OK, but really doesn't help all that much.

r=me because there's little harm here. But I don't really like the idea of the comment in KJSProxy::clear() mentioning FrameLoader::clear().

[Eric Seidel (no email)]

Comment on attachment 18431 [details]
Reset KJSProxy::m_handlerLineno in KJSProxy::clear

I'd rather fix this in a better way. And fix it for XML at the same time.  Clearing review flag.

[Alexey Proskuryakov]

I think that this was quietly fixed (in a different way) in r34273.