HTML5 Parser: Fix fast/profiler tests that depend on event handler line numbers

• Component: New Bugs
• Status: RESOLVED FIXED

[Tony Gentilcore]

HTML5 Parser: Fix fast/profiler tests that depend on event handler line numbers

[Tony Gentilcore]

Created attachment 58306 [details]
Patch

[Adam Barth]

Comment on attachment 58306 [details]
Patch

WebCore/html/HTML5Tokenizer.cpp:105
 +          if (scriptController)
I'm worried that constructTreeFromToken can cause script to run synchronously, which could invalidate the |frame|, which would invalidate scriptController.  Maybe we need a private helper function to get the script controller?

if (ScriptController* scriptController = script())
    scriptController->setEventHandlerLineNumber(...)

[Adam Barth]

I'm going to spoil you, but I'm going to merge your patch to TOT.  :)

[Adam Barth]

Committed r60940: <http://trac.webkit.org/changeset/60940>

[Eric Seidel (no email)]

Woh.  Wasn't this a huge slowdown on the benchmark?  We're adding two branches and a virtual function call to the per-token loop, no?

[Eric Seidel (no email)]

Why do we need to reset the line number to 0 after each token is processed?

[Tony Gentilcore]

(In reply to comment #3)
> I'm going to spoil you, but I'm going to merge your patch to TOT.  :)

Thanks!

Man you're fast. I had the same patch ready but it didn't finish building before the get together last night.

There was one more place in Tokenizer that the script() fn could be used. I'll mail a patch for that.

[Tony Gentilcore]

(In reply to comment #6)
> Why do we need to reset the line number to 0 after each token is processed?

I haven't confirmed, but I'm nearly certain that is for the case when a script dynamically adds an element with an event handler (it shouldn't be associated w/ the line number of the parser). In any case, it seemed safest to match the old Tokenizer's behavior for now.

[Tony Gentilcore]

(In reply to comment #5)
> Woh.  Wasn't this a huge slowdown on the benchmark?  We're adding two branches and a virtual function call to the per-token loop, no?

Can you point me to the benchmark to run?

[Adam Barth]

> Can you point me to the benchmark to run?

http://trac.webkit.org/browser/trunk/WebCore/benchmarks/parser/html-parser.html

[Eric Seidel (no email)]

(In reply to comment #8)
> (In reply to comment #6)
> > Why do we need to reset the line number to 0 after each token is processed?
> 
> I haven't confirmed, but I'm nearly certain that is for the case when a script dynamically adds an element with an event handler (it shouldn't be associated w/ the line number of the parser). In any case, it seemed safest to match the old Tokenizer's behavior for now.

Some elements carry a createdByParser flag.  But I guess not all.  Otherwise that would be easy to check after the fact.