481612 - Should send activity distributor notifications also for STATUS_RECEIVING_FROM

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/src/nsHttpTransaction.cpp#355
355     // nsHttpChannel synthesizes progress events in OnDataAvailable
356     if (status == nsISocketTransport::STATUS_RECEIVING_FROM)
357         return;
358 
359     if (mActivityDistributor) {
360         // upon STATUS_WAITING_FOR; report request body sent
361         if ((mHasRequestBody) &&
362             (status == nsISocketTransport::STATUS_WAITING_FOR))
363             mActivityDistributor->ObserveActivity(
364                 mChannel,
365                 NS_HTTP_ACTIVITY_TYPE_HTTP_TRANSACTION,
366                 NS_HTTP_ACTIVITY_SUBTYPE_REQUEST_BODY_SENT,
367                 LL_ZERO, LL_ZERO, EmptyCString());
368 
369         // report the status and progress
370         mActivityDistributor->ObserveActivity(
371             mChannel,
372             NS_HTTP_ACTIVITY_TYPE_SOCKET_TRANSPORT,
373             static_cast<PRUint32>(status),
374             LL_ZERO,
375             progress,
376             EmptyCString());
377     }

So this code doesn't call ObserveActivity for receiving_from, and nsHttpChannel doesn't do it either when synthesizing the status messages. So, this code should do the receiving_from check after sending the notification.

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

report+patch was by Bryan McQuade, r+sr=me

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

http://hg.mozilla.org/mozilla-central/rev/b145e6658509

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 365634 [details] [diff] [review]
patch

I'd like to get this into ff 3.1 as well, it is a pretty safe patch (no change in behaviour unless you install an extension that makes use of this); it just moves around a bit of code.

it allows extensions to keep track of the exact timing of receiving data, so that they can tell web developers more details about the performance of their web pages.

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

Note that Firebug and Livehttpheaders do not use this interface and thus can't be adversely affected by this change.

Comment 5

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 365634 [details] [diff] [review]
patch

a191=beltzner; biesi did a search and discovered no add-ons that use this method, so looks ok, compat-wise.

biesi: it'd be grand if you could add tests for this and/or ensure that it's a documented method on MDC.

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

Pushed to 1.9.1
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/c6d941dfc5a3

Writing a test is difficult; doing this in JS asserts in some bad ways, making the test fail.

Comment 7

[Aakash Desai [:aakashd]]

If that's true, how can this be verified? Curious is all :)