Closed
Bug 96178
Opened 23 years ago
Closed 9 years ago
implement http response-header-change observer mechanism
Categories
(Core :: Networking: HTTP, defect)
Core
Networking: HTTP
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: darin.moz, Unassigned)
Details
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•23 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla0.9.5
Reporter | ||
Comment 2•23 years ago
|
||
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.
Reporter | ||
Comment 3•23 years ago
|
||
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.
Reporter | ||
Comment 4•23 years ago
|
||
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
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
Comment 10•23 years ago
|
||
+ 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())
Reporter | ||
Comment 11•18 years ago
|
||
-> default owner
Assignee: darin → nobody
Status: ASSIGNED → NEW
QA Contact: tever → networking.http
Target Milestone: Future → ---
Updated•18 years ago
|
Priority: P4 → --
Comment 12•17 years ago
|
||
Is this bug still relevant? nsIHttpNotify is long gone...
Updated•9 years ago
|
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.
Description
•