Bug 84791

Summary: REGRESSION(r115099): html5lib/runner.html crashes.
Product: WebKit Reporter: Andreas Kling <kling>
Component: WebCore Misc.Assignee: Andreas Kling <kling>
Status: RESOLVED FIXED    
Severity: Normal CC: darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch andersca: review+

Description Andreas Kling 2012-04-24 15:45:15 PDT
Assertion below WebCore::HTMLTreeBuilder::attributesForIsindexInput().
Comment 1 Andreas Kling 2012-04-24 15:47:13 PDT
Created attachment 138671 [details]
Patch
Comment 2 Andreas Kling 2012-04-24 15:57:45 PDT
Committed r115127: <http://trac.webkit.org/changeset/115127>
Comment 3 Darin Adler 2012-04-24 17:53:59 PDT
Comment on attachment 138671 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=138671&action=review

> Source/WebCore/ChangeLog:9
> +        It's perfectly safe to remove elements from a Vector while iterating over it.

Sure, it’s safe if you do it correctly.

> Source/WebCore/html/parser/HTMLTreeBuilder.cpp:571
>      for (unsigned i = 0; i < attributes.size(); ++i) {
>          const QualifiedName& name = attributes.at(i).name();
>          if (name.matches(nameAttr) || name.matches(actionAttr) || name.matches(promptAttr))
> -            indicesToRemove.append(i);
> +            attributes.remove(i);
>      }

If two attributes in a row are name and action, this will return only name, but not action.

You need to subtract one from i when you remove the element at i so we process the new attribute that is now at index i.
Comment 4 Andreas Kling 2012-04-24 18:11:44 PDT
(In reply to comment #3)
> (From update of attachment 138671 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=138671&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        It's perfectly safe to remove elements from a Vector while iterating over it.
> 
> Sure, it’s safe if you do it correctly.

Yeah, that was a silly mistake. I should know better than to commit when I'm sleepy. :(
I ended up rolling the whole thing out so it can be relanded by the original author without piling patches on top.
Comment 5 Darin Adler 2012-04-24 19:52:42 PDT
(In reply to comment #4)
> I ended up rolling the whole thing out so it can be relanded by the original author without piling patches on top.

Sounds good.