New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users

Issue metadata

Status: Verified
Owner: ----
Closed: Mar 2010
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Restricted
  • Only users with Commit permission may comment.



Sign in to add a comment
link

Issue 36929: Disable the XSS auditor on 4.1

Reported by thakis@chromium.org, Feb 26 2010 Project Member

Issue description

 Issue 35723  describes how POST operations get a lot slower when the XSS auditor is active.

abarth improved the situation for the specific test in that bug from "a lot slower" to "'only' 400% 
slower than chrome 3".

Now, the test in that bug is not representative from normal-day usage. Then again we don't have 
any tests that do POST requests, so we don't really know how bad 400% worse performance is in 
practice.

What should happen on 4.1?

* Merge abarth's patch and swallow the 400% ("Hey, it's much better than chrome 4")
* Disable the xss auditor on 4.1 (as fast as chrome 3, but less xss auditing takes place).

Discuss.
 

Comment 1 by thakis@chromium.org, Feb 26 2010

Technically, there's also

* Do nothing

And probably even more options.

Comment 2 by jam...@chromium.org, Feb 26 2010

One of the folks commenting on  issue 35723  also reported that submitting comments to 
blogger, wordpress, etc had regressed significantly between Chrome 3 and 4.  This seems 
plausible.  A moderately sized reddit comment thread has ~2500 inline event handlers so it 
would not be surprising if we had a similar perf regression on it (given that the repro page 
had 5600 inline event handlers).

I think we should disable the XSS auditor until the perf impact that users see is known and 
then set some guidelines about how much of a loading slowdown is acceptable to re-enable 
this feature.  I also think that it's pretty unhealthy for us to be turning on features that 
hurt performance without tracking how large that performance impact is.

Comment 3 by abarth@chromium.org, Feb 26 2010

I wish we had real-world numbers to work with here.  Should we try the reddit
experiment (comparing the new fix with --disable-xss-auditor)?

FWIW, I was very careful to ensure that the auditor didn't regress any of the perf
metrics we track.  Maybe we need some kind of UMA-based catch-all metric to avoid
situations like this in the future?

Comment 4 by thakis@chromium.org, Feb 27 2010

Having data from that reddit experiment is better than having no data at all imho.

Comment 5 by mal@google.com, Feb 28 2010

Labels: OS-All ReleaseBlock-Stable ForMerge
Status: Started
Summary: Disable the XSS auditor on 4.1
We'll disable it for 4.1

Comment 6 by mal@google.com, Feb 28 2010

http://codereview.chromium.org/660254

Waiting for the branch to re-open before committing.

Comment 7 by bugdro...@gmail.com, Mar 2 2010

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=40424 

------------------------------------------------------------------------
r40424 | mal@chromium.org | 2010-03-02 12:56:02 -0800 (Tue, 02 Mar 2010) | 11 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/249/src/chrome/browser/tab_contents/render_view_host_delegate_helper.cc?r1=40424&r2=40423
   M http://src.chromium.org/viewvc/chrome/branches/249/src/chrome/common/chrome_switches.cc?r1=40424&r2=40423
   M http://src.chromium.org/viewvc/chrome/branches/249/src/chrome/common/chrome_switches.h?r1=40424&r2=40423

Disable the XSS Auditor by default for 4.1.

It causes a huge performance regression with POST.
The auditor can now be enabled by passing --enable-xss-auditor.

R= abarth
BUG=  http://crbug.com/36929 
TEST= http://www/~thakis/cgi-bin/35723.php  Click the button, and the page should
reload quickly.

Review URL: http://codereview.chromium.org/660254
------------------------------------------------------------------------

Comment 8 by mal.chro...@gmail.com, Mar 2 2010

Status: Fixed
Fixed. Should be in 4.1.249.x, x>1021.

Comment 9 by dhw@chromium.org, Mar 2 2010

Hi Mark, I hope you mean x>=1021, or else the current Beta 4.1.249.1021 doesn't have this?

Comment 10 by jam...@chromium.org, Mar 2 2010

The current beta 4.1.249.1021 does not have this fix.  We caught it too late.

Comment 11 by *mdu@chromium.org, Mar 4 2010

Status: Verified
Verified in build 4.1.249.1025 (Official Build 40600) with
http://www/~thakis/cgi-bin/35723.php, Click on the button now reloads the page quickly.

Comment 12 by krud...@gmail.com, Mar 6 2010

Great job chrome developers!
This is a good example why Chrome will win and IE loose the browser "war".

One thing I notice though, is this (please bear with me for posting this here):
When running the repro found here: http://www/~thakis/cgi-bin/35723.php (well I do not have 
access to that online repro, but I have the original file from the orig. issue)
And then clicking the button, after fist installing this version:
4.1.249.1025

The loading of the page is super fast (due to the disabled XSS editor).
But I also notice that each time I click the button in that repro, then my harddrive is beeing 
"taxed" A LOT MORE comparede to when I use this version: 3.0.195.3

Could someone here please tell me why that is happening? And if there is something else I can 
disable to prevent my hard drive beeing taxed this much?

Comment 13 by kenstuar...@gmail.com, Mar 21 2010

This change (disabling XSS Auditor) appears to make a huge difference in performance on 
older PCs, ie slow CPU speeds and low physical memory.
Thanks for making the change, and please verify performance before restoring it.
(In fact, there were some comments on the original release blog making the case that 
the code is insufficient to solve the security problem anyway.)

Comment 14 by bugdroid1@chromium.org, Oct 12 2012

Project Member
Labels: Restrict-AddIssueComment-Commit
Owner: ----
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.

Comment 15 by bugdroid1@chromium.org, Mar 10 2013

Project Member
Labels: -Area-Internals Cr-Internals

Sign in to add a comment