Closed
Bug 135046
Opened 22 years ago
Closed 22 years ago
Fix performance hit that we were taking for p3p
Categories
(Core :: Networking: Cookies, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.0
People
(Reporter: morse, Assigned: morse)
References
Details
(Whiteboard: [ADT1])
Attachments
(1 file, 1 obsolete file)
|
1.84 KB,
patch
|
dp
:
review+
waterson
:
superreview+
asa
:
approval+
|
Details | Diff | Splinter Review |
Bug 129101 implemented some enhanced p3p cookie-management features. Unfortunately it also introduced a performance hit on window-open time when p3p is turned on. This bug is to remove that performance hit.
|
Assignee | |
Comment 1•22 years ago
|
||
The way the p3p icon was implemented was that each time a window was opened, a test was made to determine if the icon should be visible. That test was being done in the cookie overlay for the status bar. The status bar does not yet exist at the time that the javascript in the overlay is executed, so the test needed to be delayed. The method used to delay the test was to use a setTimeout call. And it appears that that was causing the performance hit randomly -- that is, for the most part windows were being open in the usual amount of time, but a few times the time was significantly increased (enough so to increase the median time by 15%). The fix is to not use setTimeout, but rather to add another routine to the onload handler of the parent window (navigator.xul already has a handler there). Attaching patch that makes this change. This patch has been tested on the tinderbox comet machine which was the one machine on which we were able to see the performance hit. With this patch, the comet Txul numbers were back in the 480 ms range rather than in the 560 range.
|
|
Assignee | |
Comment 2•22 years ago
|
||
|
||
Comment 3•22 years ago
|
||
Looks good, but it may be cleaner to use DOM Events to do this (instead of
munging the `onload' string:
<html>
<head>
<script>
var listener = {
handleEvent: function(event) {
alert("hello!");
}
};
addEventListener("load", listener, false);
</script>
</head>
<body onload="alert('blah!');">
</body>
</html>|
|
Assignee | |
Comment 4•22 years ago
|
||
Attachment #77385 -
Attachment is obsolete: true
|
|
||
Comment 5•22 years ago
|
||
Comment on attachment 77389 [details] [diff] [review] using method described by waterson sr=waterson. great!
Attachment #77389 -
Flags: superreview+
|
||
Comment 6•22 years ago
|
||
Comment on attachment 77389 [details] [diff] [review] using method described by waterson Cool. That should take care of window open.
Attachment #77389 -
Flags: review+
|
||
Comment 7•22 years ago
|
||
Has this been checked in yet? What's left to do?
|
|
||
Comment 8•22 years ago
|
||
Please update this bug with an [adt1] - [adt3] impact rating (or take it off the list if it doesn't even rate adt3.) Thanks!
|
|
Assignee | |
Comment 9•22 years ago
|
||
For starters, the bug is waiting for approval from drivers.
|
||
Comment 10•22 years ago
|
||
Comment on attachment 77389 [details] [diff] [review] using method described by waterson a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #77389 -
Flags: approval+
|
||
Comment 11•22 years ago
|
||
Adding ADT1 to Status Whiteboard, nsbeta1+ and adt1.0.0.
|
||
Comment 12•22 years ago
|
||
Adding adt1.0.0+ for adt approval to check in.
|
|
Assignee | |
Comment 13•22 years ago
|
||
*** Bug 133624 has been marked as a duplicate of this bug. ***
|
|
Assignee | |
Comment 14•22 years ago
|
||
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•