Last Comment Bug 703855 - performance timing tests involving document.open fail
: performance timing tests involving document.open fail
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla17
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
http://w3c-test.org/webperf/tests/app...
: 712561 754310 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-11-19 02:39 PST by Boris Zbarsky [:bz]
Modified: 2012-08-09 04:51 PDT (History)
8 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Don't update performance timing or navigation timing state during document.open. (14.67 KB, patch)
2012-07-31 14:43 PDT, Boris Zbarsky [:bz]
jst: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2011-11-19 02:39:30 PST
See W3C test in url field.

Igor, Olli, can one of you please look into this?
Comment 1 Igor Bazarny 2011-11-21 06:04:12 PST
I looked into this problem a bit, it's not easy to tell events caused by document.open and subsequent calls from the normal load. To fix the issue we might let to register each event in the NavigationTiming only once, so that load process caused by document.open does not affect collected data.
Comment 2 Boris Zbarsky [:bz] 2011-12-21 11:14:48 PST
*** Bug 712561 has been marked as a duplicate of this bug. ***
Comment 3 Jonas Sicking (:sicking) PTO Until July 5th 2012-05-11 09:30:24 PDT
*** Bug 754310 has been marked as a duplicate of this bug. ***
Comment 4 Josh Aas 2012-05-15 10:25:08 PDT
Can we get an owner here? Is Igor planning to fix this?
Comment 5 Igor Bazarny 2012-05-16 06:14:23 PDT
Sorry, I don't have capacity right now
Comment 6 Boris Zbarsky [:bz] 2012-05-22 21:57:11 PDT
I just read through the navigation timing processing model pretty carefully, and I think this test is wrong.  Or the spec is wrong.  One or the other.  Posted http://lists.w3.org/Archives/Public/public-web-perf/2012May/0122.html
Comment 7 Boris Zbarsky [:bz] 2012-07-31 13:20:15 PDT
The spec has been clarified to match the test.
Comment 8 Boris Zbarsky [:bz] 2012-07-31 14:43:24 PDT
Created attachment 647685 [details] [diff] [review]
Don't update performance timing or navigation timing state during document.open.
Comment 9 Boris Zbarsky [:bz] 2012-07-31 14:46:02 PDT
Oh, sicking and smaug, if you think you're a better reviewer for this, please feel free to steal!
Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2012-08-08 00:16:55 PDT
Comment on attachment 647685 [details] [diff] [review]
Don't update performance timing or navigation timing state during document.open.

Sorry for the delay, looks all good! r=jst
Comment 12 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 21:59:40 PDT
Sorry, backed out in

https://hg.mozilla.org/integration/mozilla-inbound/rev/e3e9040a6eb9

for two consecutive new Linux64 Opt M2 failures

352 ERROR TEST-UNEXPECTED-FAIL | /tests/docshell/test/test_bug570341.html | Expected domContentLoadedEventStart to happen before domContentLoadedEventEnd, got domContentLoadedEventStart = 1344485549757, domContentLoadedEventEnd = 0
Comment 13 Boris Zbarsky [:bz] 2012-08-08 22:43:07 PDT
Er, yes.  That's because the constructor wasn't setting the booleans for those two.  Damn castability of boolean to int...

Relanded with that fixed as https://hg.mozilla.org/integration/mozilla-inbound/rev/cc595773e189
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-08 22:47:24 PDT
If it makes you feel any better, I spent about an hour today chasing down a bug that ended up being passing (bool, int) when I meant (int, bool), with no warning :/.
Comment 15 Ed Morley [:emorley] 2012-08-09 04:51:48 PDT
https://hg.mozilla.org/mozilla-central/rev/cc595773e189

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