694612 - window.performance is null in a webpage loaded via <object>

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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 [:bzbarsky]]

Ideally, we would just set mLoadType to LOAD_NORMAL somehow before making the OpenChannel call here, right?  Olli?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

Sounds right.

Comment 3

[Boris Zbarsky [:bzbarsky]]

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]

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 [:bzbarsky]]

> 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]

Attachment: 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 [:bzbarsky]]

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 [:bzbarsky]]

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]

Attachment: 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 [:bzbarsky]]

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 [:bzbarsky]]

Oh, this needs a testcase, but I guess I can just write one....

Comment 12

[Igor Bazarny]

Thanks, I don't have experience writing tests for <object> tags.

Comment 13

[Boris Zbarsky [:bzbarsky]]

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

Comment 14

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/e0f41e430ac0