[Webkit-unassigned] [Bug 51311] SegmentedString should provide column position

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 21 14:57:35 PST 2010


https://bugs.webkit.org/show_bug.cgi?id=51311





--- Comment #6 from Peter Rybin <peter.rybin at gmail.com>  2010-12-21 14:57:35 PST ---
> > 3. I remember Adam was concerned about forks that appeared inside advance* methods. I'm afraid I don't see how I could do without them.
> 
> Can you run the benchmark with and without your patch so we can see how much performance we're talking about?  Per-character branches usually are visible on the benchmark.

I ran 3 times before and after, and in average I have:
4354 (base) and 4377 (with patch). It's definitely visible, but I'm not sure I see a really better solution.

> > 4. HTMLTreeBuilder uses m_parser new field, but it has to upcast in order to call textPosition. I don't see how to make it better.
> 
> Can't you just pass in the textPosition instead of passing in the whole object?

I'm sorry I don't quite see how I can pass it other way. Now I do a callback from tree builder as deep as to SegmentedString when I need a column value (at script tag only).

Passing it directly from parser as a parameter might be a bit messy. Here's a stack I would have to go through:
        WebCore::HTMLTreeBuilder::processScriptStartTag()
        WebCore::HTMLTreeBuilder::processStartTagForInHead()
        WebCore::HTMLTreeBuilder::processStartTag()
        WebCore::HTMLTreeBuilder::processToken()
        WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken()
        WebCore::HTMLTreeBuilder::constructTreeFromToken()

Alternatively I can create a variable somewhere, pass it by reference into the tree builder and update this reference on each token in parser. It seems to be quite fragile to me.

Do I miss something?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.



More information about the webkit-unassigned mailing list