Bug 113003 - Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
Summary: Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Benjamin Poulain
URL:
Keywords:
: 111892 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-03-21 22:12 PDT by Benjamin Poulain
Modified: 2013-03-22 20:39 PDT (History)
6 users (show)

See Also:

Attachments
Patch (10.46 KB, patch)
2013-03-21 22:35 PDT, Benjamin Poulain 		no flags Details | Formatted Diff | Diff
Patch (12.53 KB, patch)
2013-03-22 15:30 PDT, Benjamin Poulain 		eric: review+
Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Benjamin Poulain 2013-03-21 22:12:42 PDT 
Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
Comment 1 Benjamin Poulain 2013-03-21 22:35:27 PDT 
Created attachment 194443 [details]
Patch
Comment 2 Benjamin Poulain 2013-03-22 15:30:57 PDT 
Created attachment 194645 [details]
Patch
Comment 3 Benjamin Poulain 2013-03-22 15:33:37 PDT 
*** Bug 111892 has been marked as a duplicate of this bug. ***
Comment 4 Eric Seidel (no email) 2013-03-22 16:26:00 PDT 
Comment on attachment 194645 [details]
Patch

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

LGTM!

> Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp:98
> +    return equalIgnoringCase(token.data().asStringImpl(), SVGNames::foreignObjectTag.localName().impl());

These are known to never be null, btw.
Comment 5 Benjamin Poulain 2013-03-22 16:30:20 PDT 
> > Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp:98
> > +    return equalIgnoringCase(token.data().asStringImpl(), SVGNames::foreignObjectTag.localName().impl());
> 
> These are known to never be null, btw.

I was not sure so I took the safe option :).
I will change to the non-null form before landing.

Thank you for the review.
Comment 6 Eric Seidel (no email) 2013-03-22 16:31:58 PDT 
I believe the HTMLParser basically never has any null strings ever. :)  Everything is empty at most.  In the case of a tag name, the parser will tread <> as a characters in a character token, not as a tag with empty name.

Thanks for fixing!
Comment 7 Benjamin Poulain 2013-03-22 18:38:08 PDT 
Committed r146702: <http://trac.webkit.org/changeset/146702>
Comment 8 Ryosuke Niwa 2013-03-22 19:44:13 PDT 
This patch broke Windows build:

http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/46465/steps/compile-webkit/logs/stdio

7>WebKitSystemInterface.lib(WebKitSystemInterface.obj) : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance
7>   Creating library C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\lib\WebKit.lib and object C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\lib\WebKit.exp
7>WebKit.exp : error LNK2001: unresolved external symbol "bool __cdecl WTF::equalIgnoringCase(class WTF::StringImpl *,unsigned char const *)" (?equalIgnoringCase@WTF@@YA_NPAVStringImpl@1@PBE@Z)
7>C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\bin\WebKit.dll : fatal error LNK1120: 1 unresolved externals
Comment 9 Benjamin Poulain 2013-03-22 20:32:57 PDT 
(In reply to comment #8)
> This patch broke Windows build:

I am on it.
Comment 11 Benjamin Poulain 2013-03-22 20:39:02 PDT 
(In reply to comment #10)
> Fixed in http://trac.webkit.org/changeset/146705 and http://trac.webkit.org/changeset/146710.

Thanks for fixing this.