Bug 111957 - Make sure that CSSSelector::setValue() is never called after parsing its pseudoType().
Summary: Make sure that CSSSelector::setValue() is never called after parsing its pseu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-10 23:49 PDT by Hayato Ito
Modified: 2013-03-11 03:38 PDT (History)
7 users (show)

See Also:


Attachments
Add an assertion. (1.44 KB, patch)
2013-03-10 23:52 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
Add an assertion. (1.43 KB, patch)
2013-03-10 23:56 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
Patch for landing (1.51 KB, patch)
2013-03-11 01:06 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
Patch for landing (1.51 KB, patch)
2013-03-11 01:15 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2013-03-10 23:49:05 PDT
It'd be nice to have an assertion here since CSSParserSelector::pseudoType() has an side effect.
Comment 1 Hayato Ito 2013-03-10 23:52:56 PDT
Created attachment 192408 [details]
Add an assertion.
Comment 2 Hayato Ito 2013-03-10 23:56:15 PDT
Created attachment 192409 [details]
Add an assertion.
Comment 3 Hajime Morrita 2013-03-11 00:55:48 PDT
Comment on attachment 192409 [details]
Add an assertion.

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

> Source/WebCore/ChangeLog:9
> +        CSSSelector::pseudoType() has an side effect.

Could you elaborate?
Tightening ASSERT()ion up is fine in general.
But it's hard to see the connectio between pseudoType() and setValue(), and ASSERT() and the side effect.
Comment 4 Hayato Ito 2013-03-11 00:59:17 PDT
Thank you for the review.

Let me land this after adding more description.


(In reply to comment #3)
> (From update of attachment 192409 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192409&action=review
> 
> > Source/WebCore/ChangeLog:9
> > +        CSSSelector::pseudoType() has an side effect.
> 
> Could you elaborate?
> Tightening ASSERT()ion up is fine in general.
> But it's hard to see the connectio between pseudoType() and setValue(), and ASSERT() and the side effect.
Comment 5 Hayato Ito 2013-03-11 01:06:42 PDT
Created attachment 192418 [details]
Patch for landing
Comment 6 Hayato Ito 2013-03-11 01:15:35 PDT
Created attachment 192420 [details]
Patch for landing
Comment 7 WebKit Review Bot 2013-03-11 03:38:34 PDT
Comment on attachment 192420 [details]
Patch for landing

Clearing flags on attachment: 192420

Committed r145352: <http://trac.webkit.org/changeset/145352>
Comment 8 WebKit Review Bot 2013-03-11 03:38:38 PDT
All reviewed patches have been landed.  Closing bug.