Last Comment Bug 694612 - window.performance is null in a webpage loaded via <object>
: window.performance is null in a webpage loaded via <object>
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla11
Assigned To: Igor Bazarny
:
Mentors:
data:text/html,<object width="100" he...
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-14 10:58 PDT by Boris Zbarsky [:bz]
Modified: 2011-11-17 03:23 PST (History)
4 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Assume LOAD_NORMAL if load type is not set for NavigationTiming nav type (2.17 KB, patch)
2011-11-03 06:53 PDT, Igor Bazarny
no flags Details | Diff | Splinter Review
Collect timing data when load type is not initialized (assume LOAD_NORMAL) (1.74 KB, patch)
2011-11-16 05:11 PST, Igor Bazarny
bzbarsky: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2011-10-14 10:58:27 PDT
The problem is that we end up in nsDocShell::OnStateChange with this callstack:

#0  nsDocShell::OnStateChange (this=0x119b82400, aProgress=0x119b82428, aRequest=0x118bd5058, aStateFlags=983041, aStatus=0) at ../../../mozilla/docshell/base/nsDocShell.cpp:5896
...
#5  0x0000000101ccd28b in nsLoadGroup::AddRequest (this=0x118b6c830, request=0x118bd5058, ctxt=0x0) at ../../../../mozilla/netwerk/base/src/nsLoadGroup.cpp:615
#6  0x0000000102dba858 in nsURILoader::OpenChannel (this=0x119d46200, channel=0x118bd5058, aFlags=2, aWindowContext=0x119b82430, aChannelIsOpen=true, aListener=0x11b055148) at ../../../mozilla/uriloader/base/nsURILoader.cpp:910
#7  0x0000000102dba9fc in nsURILoader::OpenChannel (this=0x119d46200, channel=0x118bd5058, aFlags=2, aWindowContext=0x119b82430, aListener=0x11b055148) at ../../../mozilla/uriloader/base/nsURILoader.cpp:936
#8  0x00000001023a0128 in nsObjectLoadingContent::OnStartRequest (this=0x11b0550e0, aRequest=0x118bd5058, aContext=0x0) at ../../../../mozilla/content/base/src/nsObjectLoadingContent.cpp:687

and at this point mLoadType == 0, so we skip the MaybeInitTiming call.

Why do we have this mLoadType check anyway?
Comment 1 Boris Zbarsky [:bz] 2011-10-14 11:13:50 PDT
Ideally, we would just set mLoadType to LOAD_NORMAL somehow before making the OpenChannel call here, right?  Olli?
Comment 2 Olli Pettay [:smaug] 2011-10-14 14:50:07 PDT
Sounds right.
Comment 3 Boris Zbarsky [:bz] 2011-10-18 20:07:37 PDT
So wait.  I still don't understand why this mLoadType check is there.  (Also, let's ignore the fact that treating STATE_START in the subdocshell there as a navigation start is bogus for <object>.).

Why can't we just unconditionally MaybeInitTiming()?
Comment 4 Igor Bazarny 2011-10-19 07:26:45 PDT
Some tests which loaded xml (non-html) content via object tag got broken (crashed IIRC), there's indirect reference to that problem in comment 92 to issue 570341 https://bugzilla.mozilla.org/show_bug.cgi?id=570341#c92. There was some issue down the line with mLoadType needed to derive navigation type. And load type was not set up for objects.
Related: https://bugzilla.mozilla.org/show_bug.cgi?id=666897
Comment 5 Boris Zbarsky [:bz] 2011-10-19 12:52:16 PDT
> Some tests which loaded xml (non-html) content via object tag got broken

Was the issue just that mNavigationStartTimeStamp never got initialized and then you hit asserts about that?  That presumably needs to be addressed even if mLoadType is set to something nonzero.

> There was some issue down the line with mLoadType needed to derive navigation type

That code could just map 0 to LOAD_NORMAL if desired.

> And load type was not set up for objects.

Yes, which means that .performance is just broken in <object>, hence this bug...
Comment 6 Igor Bazarny 2011-11-03 06:53:09 PDT
Created attachment 571634 [details] [diff] [review]
Assume LOAD_NORMAL if load type is not set for NavigationTiming nav type


Not sure whether I can peek into <object> and don't have much time right now. so patch includes just fix in navigation timing that lets it collect data if mLoadType was not initialized which is the case for object tag. Related W3C tests now pass
Comment 7 Boris Zbarsky [:bz] 2011-11-03 07:30:23 PDT
Comment on attachment 571634 [details] [diff] [review]
Assume LOAD_NORMAL if load type is not set for NavigationTiming nav type

Do you really mean to have two NotifyFetchStart calls there, one conditioned on this == aProgress and one not?  Looks like some sort of merge fail...
Comment 8 Boris Zbarsky [:bz] 2011-11-16 02:43:18 PST
Igor, are you actively working on this?  If not, should I just take this?  This bug is causing us to fail most of the W3C navigation timing test suite....
Comment 9 Igor Bazarny 2011-11-16 05:11:03 PST
Created attachment 574879 [details] [diff] [review]
Collect timing data when load type is not initialized (assume LOAD_NORMAL)

Sorry for the delay, was distracted by other things.

Not sure this is the best way to go, I don't have enough expertise to look into the <object> loading code and make sure that load type is set there.
Comment 10 Boris Zbarsky [:bz] 2011-11-16 05:13:32 PST
Comment on attachment 574879 [details] [diff] [review]
Collect timing data when load type is not initialized (assume LOAD_NORMAL)

I think this is the right approach.  Getting data from the object loading code to here is pretty hard...
Comment 11 Boris Zbarsky [:bz] 2011-11-16 05:17:17 PST
Oh, this needs a testcase, but I guess I can just write one....
Comment 12 Igor Bazarny 2011-11-16 06:33:25 PST
Thanks, I don't have experience writing tests for <object> tags.
Comment 14 Marco Bonardo [::mak] 2011-11-17 03:03:13 PST
https://hg.mozilla.org/mozilla-central/rev/e0f41e430ac0

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