Last Comment Bug 340537 - 7% Tdhtml regression on June 5
: 7% Tdhtml regression on June 5
: fixed1.8.0.5, fixed1.8.1, regression
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Linux
: P2 normal (vote)
: mozilla1.8.1beta1
Assigned To: Blake Kaplan (:mrbkap)
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Depends on:
Blocks: 339918
  Show dependency treegraph
Reported: 2006-06-06 09:04 PDT by Boris Zbarsky [:bz] (still a bit busy)
Modified: 2006-11-10 12:10 PST (History)
5 users (show)
mtschrep: blocking1.8.1+
dveditz: blocking1.8.0.5+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Potential fix (2.76 KB, patch)
2006-06-06 14:08 PDT, Blake Kaplan (:mrbkap)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.0.5+
mtschrep: approval1.8.1+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] (still a bit busy) 2006-06-06 09:04:56 PDT

Checkin range:

If I were a betting man, I'd bet on bug 339918.  With that change, once we have a single toplevel script access on a window (before any accesses from functions), all following accesses on that window will do security checks, as far as I can tell.
Comment 1 Blake Kaplan (:mrbkap) 2006-06-06 14:08:46 PDT
Created attachment 224604 [details] [diff] [review]
Potential fix

This patch attempts to undo some of the damage by not really caching the value if we were unable to find a function object on the stack (so we'll do the security checks for top level scripts, but only cache the result when we actually found a function object). This might still ding perf on scripts who only access window or document properties from top level scripts, but I'm hoping it will save Tdhtml setInterval functions, etc.
Comment 2 Blake Kaplan (:mrbkap) 2006-06-06 16:48:01 PDT
I checked the potential fix into the trunk. All tinderboxes running Tdhtml except for argo and argo-vm showed drops back to pre-bug 339918 numbers. I'm marking this bug as fixed, though I cannot explain argo's reaction.
Comment 3 Blake Kaplan (:mrbkap) 2006-07-05 14:31:00 PDT
Nominating to match bug 339918.
Comment 4 Blake Kaplan (:mrbkap) 2006-07-05 14:31:42 PDT
Comment on attachment 224604 [details] [diff] [review]
Potential fix

This is a companion patch for bug 339918.
Comment 5 Blake Kaplan (:mrbkap) 2006-07-06 12:49:04 PDT
Fix checked into the 1.8 branch.
Comment 6 Daniel Veditz [:dveditz] 2006-07-06 13:30:45 PDT
Comment on attachment 224604 [details] [diff] [review]
Potential fix

approved for 1.8.0 branch, a=dveditz for drivers
Comment 7 Blake Kaplan (:mrbkap) 2006-07-06 15:13:58 PDT
Fix checked into the 1.8.0 branch.

Note You need to log in before you can comment on or make changes to this bug.