XSSAuditor performance regression in M26

XSSAuditor performance regression in M26

The refactoring we did for the threaded parser introduced a performance
regression in innerHTML because we'd boot up the XSSAuditor for fragment
parsing. This CL returns to our earlier behavior of not booting up the
XSSAuditor when parsing fragments.

BUG=230504
TBR=eseidel

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=148792

[eseidel, 2013-04-17 21:05:27]

How did you perf test this?  This looks great otherwise.

[abarth-chromium, 2013-04-17 21:22:44]

On 2013/04/17 21:05:27, Eric Seidel (Google) wrote:
> How did you perf test this?  This looks great otherwise.

I haven't perf tested it yet because I'm having trouble with my local build.  My
plan is to run
https://chromium.googlesource.com/chromium/blink/+/master/PerformanceTests/Pa...
from a web server (with a query string) before landing.

[abarth-chromium, 2013-04-21 17:13:33]

I've confirmed that this CL is a speedup on tiny-innerHTML.html when run from a
web server.

$ PerformanceTests
$ python -m SimpleHTTPServer
[...]
$ out/Release/DumpRenderTree.app/Contents/MacOS/DumpRenderTree
http://127.0.0.1:8000/Parser/tiny-innerHTML.html?foo=bar,baz

Speedup from this CL: 3.38%

I'd expect the speedup to become bigger the longer the query string and the more
POST data.

@Eric: Would you be willing to review this CL?  Thanks!

[abarth-chromium, 2013-04-21 19:07:36]

Eric gave this an LGTM over email.  I'm going to mark it as TBR=eseidel to make
the tools happy.  Sounds like rietveld doesn't work well on mobile.

[abarth-chromium, 2013-04-21 19:08:18]

Committed patchset #1 manually as r148792 (presubmit successful).

[tonyg, 2013-04-22 16:06:00]

lgtm

Thanks for taking care of this. Just one question to consider if you feel like
making a follow-up.

https://codereview.chromium.org/14327009/diff/1/Source/core/html/parser/XSSAu...
File Source/core/html/parser/XSSAuditor.cpp (right):

https://codereview.chromium.org/14327009/diff/1/Source/core/html/parser/XSSAu...
Source/core/html/parser/XSSAuditor.cpp:337: if (!m_isEnabled || m_xssProtection
== ContentSecurityPolicy::AllowReflectedXSS)
Would it be cleaner to flip the order of the !m_isEnabled and
m_state==Initialized checks here rather than to lie about m_state in
initForFragment()?

[abarth-chromium, 2013-04-22 18:14:55]

https://codereview.chromium.org/14327009/diff/1/Source/core/html/parser/XSSAu...
File Source/core/html/parser/XSSAuditor.cpp (right):

https://codereview.chromium.org/14327009/diff/1/Source/core/html/parser/XSSAu...
Source/core/html/parser/XSSAuditor.cpp:337: if (!m_isEnabled || m_xssProtection
== ContentSecurityPolicy::AllowReflectedXSS)
On 2013/04/22 16:06:01, tonyg wrote:
> Would it be cleaner to flip the order of the !m_isEnabled and
> m_state==Initialized checks here rather than to lie about m_state in
> initForFragment()?

Yeah, that makes a lot of sense.