Closed Bug 96178 Opened 23 years ago Closed 9 years ago

implement http response-header-change observer mechanism

Categories

(Core :: Networking: HTTP, defect)

defect
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: darin.moz, Unassigned)

Details

Attachments

(1 file)

it would be nice if nsIHttpNotify implementations could know which response headers have changed when onExamineResponse is called. that way, implementors could easily filter out changes that are not interesting to them. for example, cookies would only care if a Set-Cookie header was set/modified. (BTW: response headers can "change" when the parser reads HTTP-EQUIV headers, or when the multipart stream converter detects Set-Cookie headers, etc.) to make this possible i see several solutions: 1) add a parameter to nsIHttpNotify::onExamineResponse which indicates the header modified (NULL if the response is new). 2) add a new method, such as nsIHttpNotify::onResponseHeaderModified, which takes a parameter indicating the header modified and perhaps also the new header value. 3) add a new interface which the clients may optionally implement if they are interested in "header modified" notifications. i prefer option 2, but if nsIHttpNotify is already implemented outside the mozilla tree (by embedders) then perhaps option 3 would be best.
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla0.9.5
-> future
Target Milestone: mozilla0.9.5 → Future
for bugs like bug 96178, we may actually want to allow clients to specify up front the headers they are interested in. this would significantly reduce the number of OnExamineResponse calls. however, we'd still need to notify clients of each and every out-going request.
this new API may also want to provide clients with a way to specify whether or not their observer code is thread safe. if thread safe, then we could avoid synchronously proxying to the UI thread.
correction: for bugs like bug 103745 ...
->Darin nsIHttpNotify::onExamineResponse didn't exist in the src code. I think what you point to is nsCookieHttpNotify::onExamineResponse. If you add a new method like nsIHttpNotify::onResponseHeaderModified, when client get a reponse from server, the must often to switch thread to call the method. I don't think it is a good method. I think we can decide whether to call onExamineResponse on high level. I feel we can add a decision condition in the nsHttpChannel::ProcessReponse in order to reduce the call times for nsCookieHttpNotify::onExamineReponse. In this way , the program can avoid switch threads frequently. browser-china-ptf@sun.com
Attached patch proposed patch (deleted) — Splinter Review
This patch can make program avoid frequently switching thread to call nsCookieHttpNotify::OnExamineReponse. In this way, we can enhance the performance of the http processing.
I don't think sending the notifications only for cookies is a solution to the original problem. Neither does it work for other consumers of nsIHttpNotify which by the way does have onExamineResponse (see mozilla/netwerk/protocol/http/public/nsIHttpNotify.idl)
Comment on attachment 79771 [details] [diff] [review] proposed patch based on comments above.
Attachment #79771 - Flags: needs-work+
Thanks Gagan, I forgot other consumers of nsIHttpNotify like nsP3PService
+ if (!(NS_FAILED(rv) && cookieHeader.IsEmpty())) Shouldn't that be + if (!(NS_FAILED(rv) || cookieHeader.IsEmpty())) or perhaps more readibly + if (NS_SUCCEEDED(rv) && !cookieHeader.IsEmpty())
-> default owner
Assignee: darin → nobody
Status: ASSIGNED → NEW
QA Contact: tever → networking.http
Target Milestone: Future → ---
Is this bug still relevant? nsIHttpNotify is long gone...
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: