Closed Bug 1365519 Opened 3 years ago Closed 3 years ago

nsUnknownDecoder block off-main thread delivery?

Categories

(Core :: Networking: HTTP, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: schien, Assigned: dragana)

References

Details

(Whiteboard: [necko-backlog])

Attachments

(1 file, 1 obsolete file)

nsUnknownDecoder doesn't implement nsIThreadRetargetableStreamListener, so it might block the mNextListener from requesting OMT ODA.

https://searchfox.org/mozilla-central/rev/9a7fbdee1d54f99cd548af95b81231d80e5f9ad1/netwerk/streamconv/converters/nsUnknownDecoder.cpp#120
Whiteboard: [necko-backlog]
We should fix this after PBg-Http is landed in order to trigger HTTP OMT in content process as much as possible.
Blocks: PBg-HTTP
I did not know about this bug, but I wanted to fix it with bug 1357678. I was intended to fix all streamListeners in netwerk/streamconv
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Attached patch bug_1365519.patch (obsolete) — Splinter Review
Attachment #8879539 - Flags: review?(honzab.moz)
Comment on attachment 8879539 [details] [diff] [review]
bug_1365519.patch

Review of attachment 8879539 [details] [diff] [review]:
-----------------------------------------------------------------

::: netwerk/streamconv/converters/nsUnknownDecoder.cpp
@@ +448,2 @@
>    if (channel) {
>      nsresult rv = ConvertEncodedData(aRequest, mBuffer, mBufferLen);

maybe add a short comment that this is always called only on a single thread for one instance of this object

::: netwerk/streamconv/converters/nsUnknownDecoder.h
@@ +135,5 @@
>    bool mRequireHTMLsuffix;
>  
>    nsCString mContentType;
>  
> +  mutable mozilla::Mutex mMutex;

please comment what this syncs
Attachment #8879539 - Flags: review?(honzab.moz) → review+
Attachment #8879539 - Attachment is obsolete: true
Attachment #8880480 - Flags: review+
Keywords: checkin-needed
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4878203823c
Make nsUnknownDecoder nsIThreadRetargetableStreamListener. r=mayhemer
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b4878203823c
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Depends on: 1376722
Depends on: 1377353
Depends on: 1379631
Blocks: 1410146
No longer blocks: 1410146
Depends on: 1410146
You need to log in before you can comment on or make changes to this bug.