251418 - Expose progress notification events through nsIXMLHttpRequest

Status: RESOLVED FIXED

(Core :: XML, defect)

Description

[Scott MacGregor]

An excerpt from my e-mail:

"I'm trying to hook up some progress UI for RSS Integration in Thunderbird. I
was trying to come up with a way to get the http progress event sink
notifications for my running XMLHttpRequest. I wanted to ask for your thoughts
and comments about some ways to do this.

Currently nsIXMLHttpRequest passes itself in as the interface callbacks object
to the underlying HTTP channel. It does not implement nsIProgressEventSink so
the HTTP channel currently does not broadcast any progress notifications."

Here's one approach to do this:

1) Make nsXMLHttpRequest implement nsIProgressEventSink.
2) Add a new event handler to nsIJSXMLHttpRequest called onprogress or
onprogressavailable
3) Add two new attributes to nsIXMLHttpRequest for the current progress and max
progress
4) When the xml http request gets a progress notification, it can call out
through the event handler informing the JS consumer that progress is available.
5) The JS consumer can query the two new attributes on nsIXMLHttpRequest to
calculate the progress for this request.

Comment 1

[Scott MacGregor]

Attachment: work in progress

the framework for my suggested approach after talking to Johnny. 

I'm still figuring out how to expose the current and max progress attributes
through dom event as opposed to adding attributes to nsIXMLHttpRequest

Comment 2

[Scott MacGregor]

Attachment: the fix

I also need to make packaging changes before checking in now that we build the
IDL files in dom\public\ls. We have a new xpt file that needs packaged.

Comment 3

[Johnny Stenback (:jst)]

Comment on attachment 153214 [details] [diff] [review]
the fix

- In extensions/xmlextras/base/public/nsIXMLHttpRequest.idl

+   * Meant to be a script-only mechanism for setting a progress event
listener.
+   * The attribute is expected to be JavaScript function object. When
+   * the error event occurs, the function is invoked.
+   * This attribute should not be used from native code!!
+   * This event listener will be called multiple times during the open
request.

s/will/may/ since there's no quaratee (AFAIK) cthat we'll call this more than
once on tiny files.

- In nsXMLHttpRequest::OnProgress():

+    nsCOMPtr<nsIDOMEvent> event;
+    nsEvent evt(NS_EVENT_NULL ); // what name do we make up here? 
+    CreateEvent(&evt, getter_AddRefs(event));

return if CreateEvent() fails, use NS_ENSURE_SUCCESS(rv, rv)...

+    nsXMLHttpProgressEvent * progressEvent = new nsXMLHttpProgressEvent(); 
+    if (progressEvent)
...

Return NS_ERROR_OUT_OF_MEMORY if !progressEvent in stead of nesting the code
only if progressEvent is non-null.

+      event = do_QueryInterface(NS_STATIC_CAST(nsISupports *, progressEvent)); 

Loose the cast, it's not needed.

+// DOM event class to handle progress notifications
+nsXMLHttpProgressEvent::nsXMLHttpProgressEvent()
+{}
+
+void nsXMLHttpProgressEvent::Init(nsIDOMEvent * aInner, PRUint32
aCurrentProgress, PRUint32 aMaxProgress)
+{
+  mInner = aInner; 
+  mCurProgress = aCurrentProgress;
+  mMaxProgress = aMaxProgress;
+}

Any reason not to move this code into the ctor and loose the Init() method?

- In extensions/xmlextras/build/src/nsXMLExtrasModule.cpp:

You'll need to add:

  rv = catman->AddCategoryEntry(JAVASCRIPT_DOM_CLASS,
				"XMLHttpProgressEvent",
				XMLEXTRAS_DOMCI_EXTENSION_CONTRACTID,
				PR_TRUE, PR_TRUE, getter_Copies(previous));
  NS_ENSURE_SUCCESS(rv, rv);

to RegisterXMLExtras() (even though I erronously told you that you don't need
that code, sorry), w/o that the new event class doesn't have classinfo and
you'll need to QI it etc to get to all the interfaces etc.

r+sr=jst with that changed.

Comment 4

[Scott MacGregor]

Attachment: updated patch with jst's review comments

Comment 5

[Scott MacGregor]

Comment on attachment 153237 [details] [diff] [review]
updated patch with jst's review comments

carrying forward the r/sr

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You landed this while the tree was closed...

Comment 7

[Scott MacGregor]

sorry i thought i was checking into the aviary 1.0 branch...

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

okay. I hop this lands on the trunk too, once it's open :-)

Comment 9

[Scott MacGregor]

fixed on the trunk and the aviary 1.0 branch