Closed Bug 1405506 Opened 2 years ago Closed 2 years ago

StreamFilter.disconnect does not flush data that was buffered in the child


(WebExtensions :: Request Handling, enhancement, P3)

57 Branch


(firefox57 wontfix, firefox58 verified)

Tracking Status
firefox57 --- wontfix
firefox58 --- verified


(Reporter: robwu, Assigned: kmag)




(3 files)

From the extension's perspective, when filter.suspend() is called, the extension is not expected to handle any data.
So if an extension writes previously received data and calls filter.disconnect(), then I would expect all data to be sent normally.

filter = browser.webRequest.filterResponseData(requestId);
let chunks = [];
filter.ondata = ({data}) => {

// Later:
for (let chunk of chunks) {
// Expected: All data is sent to the page.
// Actual: It is not.

When StreamFilterChild::mState == State::Suspended, mBufferedData can be non-empty (e.g. due to StreamFilterChild::RecvData [1]).

When StreamFilterChild::Disconnect is invoked, I expect any buffered data to be flushed before the channel is resumed.

However, when I follow what happens upon calling filter.disconnect(), I see:

1. StreamFilterChild::Disconnect
  -> mState = State::Disconnecting (was: Suspended)
  -> mNextState = State::Disconnected
  -> SendDisconnect()

2. StreamFilterParent::RecvDisconnect
  -> mChannel->Resume()
  -> mState = State::Disconnecting (was: Suspended)
  -> SendFlushData()

3. StreamFilterChild::RecvFlushData
  -> SendFlushedData()
  -> mState = State::Disconnected (was: Disconnecting)

4. StreamFilterParent::RecvFlushedData
  -> Destroy()
  -> StreamFilterParent::FlushBufferedData()
  -> mState = State::Disconnected
5. StreamFilterParent::FlushBufferedData
  -> Flushes mBufferedData from StreamFilterParent.

During this ping-ponging, mBufferedData in StreamFilterParent is not touched at all. Consequently, when a filter is suspended and disconnected, the first chunks of a response body may be lost.

StreamFilterChild::FlushBufferedData is the only method that removes from mBufferedData in the child (and dispatches the chunks to the extension via the filter.ondata event). This is called from:
- StreamFilterChild::Resume
  -> if mState = State::Suspended , Suspending , Resuming or TransferringData
- StreamFilterChild::SetNextState
  -> if mState = State::TransferringData (via RecvResumed)

However, I cannot do this either:
filter.resume(); // would synchronously call filter.ondata so we could call filter.write
...because StreamFilterChild::Resume puts mNextState in State::TransferringData, while StreamFilterChild::Disconnect throws when in this state.


1. Load extension at about:debugging.
2. Click on the extension button to open
3. The extension will suspend the filter on the first data event, and disconnect() after two seconds.

- The image should be displayed (remove ?disco... to see the expected image).

- Image is corrupted because parts of the response body were lost.

Note: If you get the expected result at the first try, then you have been lucky. Click the extension button again, or try to load a different URL containing "disco" that would respond with a large response body.
(In reply to Rob Wu [:robwu] from comment #0)
> During this ping-ponging, mBufferedData in StreamFilterParent is not touched

Of course I meant "mBufferedData in StreamFilterChild".

This bug also applies to the child in State::Suspending (RecvData will also buffer messages when in this state).

It seems that this bug can be solved by including the content of mBufferedData in the SendDisconnect IPC call (or just calling SendWrite for every element in mBufferedData before calling SendDisconnect in Disconnect/SetNextState).
Priority: -- → P3
Depends on: 1398120
Comment on attachment 8924034 [details]
Bug 1405506: Flush buffered data when disconnecting suspended channel.
Attachment #8924034 - Flags: review?(mixedpuppy) → review+
Assignee: nobody → kmaglione+bmo
Bug 1405506: Flush buffered data when disconnecting suspended channel. r=mixedpuppy
Blocks: 1398120
No longer depends on: 1398120
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Verified as fixed using Build identifier: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:58.0) Gecko/20100101 Firefox/58.0. Please see postfix screenshot.
Attached image Postfix screenshot
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.