Bug 1410755: Add StreamFilterParent as a request to the channel load group. r?mixedpuppy
When the base request that we are filtering finishes, it generally removes it
self from the document's load group. If that happens before the parser has
started (which tends to be the case for XML documents that haven't consumed
any data), it can unblock the load event too early. Adding the
StreamFilterParent to the load group while the request is still pending
prevents this problem.
MozReview-Commit-ID: InxAVZQy9kT
--- a/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_filterResponseData.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_filterResponseData.js
@@ -18,16 +18,116 @@ server.registerPathHandler("/redirect",
});
server.registerPathHandler("/dummy", (request, response) => {
response.setStatusLine(request.httpVersion, 200, "OK");
response.setHeader("Access-Control-Allow-Origin", "*");
response.write("ok");
});
+server.registerPathHandler("/dummy.xhtml", (request, response) => {
+ response.setStatusLine(request.httpVersion, 200, "OK");
+ response.setHeader("Content-Type", "application/xhtml+xml");
+ response.write(String.raw`<?xml version="1.0"?>
+ <html xml:lang="en" xmlns="http://www.w3.org/1999/xhtml">
+ <head/>
+ <body/>
+ </html>
+ `);
+});
+
+// Tests that the stream filter request is added to the document's load
+// group, and blocks an XML document's load event until after the filter
+// stops sending data.
+add_task(async function test_xml_document_loadgroup_blocking() {
+ let extension = ExtensionTestUtils.loadExtension({
+ background() {
+ browser.webRequest.onBeforeRequest.addListener(
+ request => {
+ let filter = browser.webRequest.filterResponseData(request.requestId);
+
+ let data = [];
+ filter.ondata = event => {
+ data.push(event.data);
+ };
+ filter.onstop = async () => {
+ browser.test.sendMessage("phase", "original-onstop");
+
+ // Make a few trips through the event loop.
+ for (let i = 0; i < 10; i++) {
+ await new Promise(resolve => setTimeout(resolve, 0));
+ }
+
+ for (let buffer of data) {
+ filter.write(buffer);
+ }
+ browser.test.sendMessage("phase", "filter-onstop");
+ filter.close();
+ };
+ }, {
+ urls: ["http://example.com/dummy.xhtml"],
+ },
+ ["blocking"]);
+ },
+
+ files: {
+ "content_script.js"() {
+ browser.test.sendMessage("phase", "content-script-start");
+ window.addEventListener("DOMContentLoaded", () => {
+ browser.test.sendMessage("phase", "content-script-domload");
+ }, {once: true});
+ window.addEventListener("load", () => {
+ browser.test.sendMessage("phase", "content-script-load");
+ }, {once: true});
+ },
+ },
+
+ manifest: {
+ permissions: [
+ "webRequest",
+ "webRequestBlocking",
+ "http://example.com/",
+ ],
+
+ content_scripts: [{
+ matches: ["http://example.com/dummy.xhtml"],
+ run_at: "document_start",
+ js: ["content_script.js"],
+ }],
+ },
+ });
+
+ await extension.startup();
+
+ const EXPECTED = [
+ "original-onstop",
+ "filter-onstop",
+ "content-script-start",
+ "content-script-domload",
+ "content-script-load",
+ ];
+
+ let done = new Promise(resolve => {
+ let phases = [];
+ extension.onMessage("phase", phase => {
+ phases.push(phase);
+ if (phases.length === EXPECTED.length) {
+ resolve(phases);
+ }
+ });
+ });
+
+ let contentPage = await ExtensionTestUtils.loadContentPage("http://example.com/dummy.xhtml");
+
+ deepEqual(await done, EXPECTED, "Things happened, and in the right order");
+
+ await contentPage.close();
+ await extension.unload();
+});
+
add_task(async function() {
let extension = ExtensionTestUtils.loadExtension({
background() {
let pending = [];
browser.webRequest.onBeforeRequest.addListener(
data => {
let filter = browser.webRequest.filterResponseData(data.requestId);
--- a/toolkit/components/extensions/webrequest/StreamFilterParent.cpp
+++ b/toolkit/components/extensions/webrequest/StreamFilterParent.cpp
@@ -113,16 +113,18 @@ StreamFilterParent::StreamFilterParent()
, mOffset(0)
, mState(State::Uninitialized)
{}
StreamFilterParent::~StreamFilterParent()
{
NS_ReleaseOnMainThreadSystemGroup("StreamFilterParent::mChannel",
mChannel.forget());
+ NS_ReleaseOnMainThreadSystemGroup("StreamFilterParent::mLoadGroup",
+ mLoadGroup.forget());
NS_ReleaseOnMainThreadSystemGroup("StreamFilterParent::mOrigListener",
mOrigListener.forget());
NS_ReleaseOnMainThreadSystemGroup("StreamFilterParent::mContext",
mContext.forget());
}
bool
StreamFilterParent::Create(dom::ContentParent* aContentParent, uint64_t aChannelId, const nsAString& aAddonId,
@@ -216,27 +218,17 @@ StreamFilterParent::CheckListenerChain()
void
StreamFilterParent::Broken()
{
AssertIsActorThread();
mState = State::Disconnecting;
- RefPtr<StreamFilterParent> self(this);
- RunOnIOThread(FUNC, [=] {
- self->FlushBufferedData();
-
- RunOnActorThread(FUNC, [=] {
- if (self->IPCActive()) {
- self->mDisconnected = true;
- self->mState = State::Disconnected;
- }
- });
- });
+ FinishDisconnect();
}
/*****************************************************************************
* State change requests
*****************************************************************************/
IPCResult
StreamFilterParent::RecvClose()
@@ -346,26 +338,40 @@ IPCResult
StreamFilterParent::RecvFlushedData()
{
AssertIsActorThread();
MOZ_ASSERT(mState == State::Disconnecting);
Destroy();
+ FinishDisconnect();
+ return IPC_OK();
+}
+
+void
+StreamFilterParent::FinishDisconnect()
+{
RefPtr<StreamFilterParent> self(this);
RunOnIOThread(FUNC, [=] {
self->FlushBufferedData();
+ RunOnMainThread(FUNC, [=] {
+ if (mLoadGroup) {
+ Unused << mLoadGroup->RemoveRequest(this, nullptr, NS_OK);
+ }
+ });
+
RunOnActorThread(FUNC, [=] {
- self->mState = State::Disconnected;
- self->mDisconnected = true;
+ if (self->mState != State::Closed) {
+ self->mState = State::Disconnected;
+ self->mDisconnected = true;
+ }
});
});
- return IPC_OK();
}
/*****************************************************************************
* Data output
*****************************************************************************/
IPCResult
StreamFilterParent::RecvWrite(Data&& aData)
@@ -402,16 +408,97 @@ StreamFilterParent::Write(Data& aData)
mOffset, aData.Length());
NS_ENSURE_SUCCESS(rv, rv);
mOffset += aData.Length();
return NS_OK;
}
/*****************************************************************************
+ * nsIRequest
+ *****************************************************************************/
+
+NS_IMETHODIMP
+StreamFilterParent::GetName(nsACString& aName)
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->GetName(aName);
+}
+
+NS_IMETHODIMP
+StreamFilterParent::GetStatus(nsresult* aStatus)
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->GetStatus(aStatus);
+}
+
+NS_IMETHODIMP
+StreamFilterParent::IsPending(bool* aIsPending)
+{
+ switch (mState) {
+ case State::Initialized:
+ case State::TransferringData:
+ case State::Suspended:
+ *aIsPending = true;
+ break;
+ default:
+ *aIsPending = false;
+ }
+ return NS_OK;
+}
+
+NS_IMETHODIMP
+StreamFilterParent::Cancel(nsresult aResult)
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->Cancel(aResult);
+}
+
+NS_IMETHODIMP
+StreamFilterParent::Suspend()
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->Suspend();
+}
+
+NS_IMETHODIMP
+StreamFilterParent::Resume()
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->Resume();
+}
+
+NS_IMETHODIMP
+StreamFilterParent::GetLoadGroup(nsILoadGroup** aLoadGroup)
+{
+ *aLoadGroup = mLoadGroup;
+ return NS_OK;
+}
+
+NS_IMETHODIMP
+StreamFilterParent::SetLoadGroup(nsILoadGroup* aLoadGroup)
+{
+ return NS_ERROR_NOT_IMPLEMENTED;
+}
+
+NS_IMETHODIMP
+StreamFilterParent::GetLoadFlags(nsLoadFlags* aLoadFlags)
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->GetLoadFlags(aLoadFlags);
+}
+
+NS_IMETHODIMP
+StreamFilterParent::SetLoadFlags(nsLoadFlags aLoadFlags)
+{
+ MOZ_ASSERT(mChannel);
+ return mChannel->SetLoadFlags(aLoadFlags);
+}
+
+/*****************************************************************************
* nsIStreamListener
*****************************************************************************/
NS_IMETHODIMP
StreamFilterParent::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext)
{
AssertIsMainThread();
@@ -424,16 +511,23 @@ StreamFilterParent::OnStartRequest(nsIRe
RunOnActorThread(FUNC, [=] {
if (self->IPCActive()) {
self->mState = State::Disconnected;
CheckResult(self->SendError(NS_LITERAL_CSTRING("Channel redirected")));
}
});
}
+ if (!mDisconnected) {
+ Unused << mChannel->GetLoadGroup(getter_AddRefs(mLoadGroup));
+ if (mLoadGroup) {
+ Unused << mLoadGroup->AddRequest(this, nullptr);
+ }
+ }
+
nsresult rv = mOrigListener->OnStartRequest(aRequest, aContext);
// Important: Do this only *after* running the next listener in the chain, so
// that we get the final delivery target after any retargeting that it may do.
if (nsCOMPtr<nsIThreadRetargetableRequest> req = do_QueryInterface(aRequest)) {
nsCOMPtr<nsIEventTarget> thread;
Unused << req->GetDeliveryTarget(getter_AddRefs(thread));
if (thread) {
@@ -481,17 +575,23 @@ StreamFilterParent::OnStopRequest(nsIReq
nsresult
StreamFilterParent::EmitStopRequest(nsresult aStatusCode)
{
AssertIsMainThread();
MOZ_ASSERT(!mSentStop);
mSentStop = true;
- return mOrigListener->OnStopRequest(mChannel, mContext, aStatusCode);
+ nsresult rv = mOrigListener->OnStopRequest(mChannel, mContext, aStatusCode);
+
+ if (mLoadGroup && !mDisconnected) {
+ Unused << mLoadGroup->RemoveRequest(this, nullptr, aStatusCode);
+ }
+
+ return rv;
}
/*****************************************************************************
* Incoming data handling
*****************************************************************************/
void
StreamFilterParent::DoSendData(Data&& aData)
@@ -693,13 +793,23 @@ StreamFilterParent::ActorDestroy(ActorDe
}
void
StreamFilterParent::DeallocPStreamFilterParent()
{
RefPtr<StreamFilterParent> self = dont_AddRef(this);
}
-NS_IMPL_ISUPPORTS(StreamFilterParent, nsIStreamListener, nsIRequestObserver, nsIThreadRetargetableStreamListener)
+NS_INTERFACE_MAP_BEGIN(StreamFilterParent)
+ NS_INTERFACE_MAP_ENTRY(nsIStreamListener)
+ NS_INTERFACE_MAP_ENTRY(nsIRequestObserver)
+ NS_INTERFACE_MAP_ENTRY(nsIRequest)
+ NS_INTERFACE_MAP_ENTRY(nsIThreadRetargetableStreamListener)
+ NS_INTERFACE_MAP_ENTRY_AGGREGATED(nsIChannel, mChannel)
+ NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIStreamListener)
+NS_INTERFACE_MAP_END
+
+NS_IMPL_ADDREF(StreamFilterParent)
+NS_IMPL_RELEASE(StreamFilterParent)
} // namespace extensions
} // namespace mozilla
--- a/toolkit/components/extensions/webrequest/StreamFilterParent.h
+++ b/toolkit/components/extensions/webrequest/StreamFilterParent.h
@@ -38,21 +38,23 @@ namespace extensions {
using namespace mozilla::dom;
using mozilla::ipc::IPCResult;
class StreamFilterParent final
: public PStreamFilterParent
, public nsIStreamListener
, public nsIThreadRetargetableStreamListener
+ , public nsIRequest
, public StreamFilterBase
{
public:
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSISTREAMLISTENER
+ NS_DECL_NSIREQUEST
NS_DECL_NSIREQUESTOBSERVER
NS_DECL_NSITHREADRETARGETABLESTREAMLISTENER
StreamFilterParent();
using ParentEndpoint = mozilla::ipc::Endpoint<PStreamFilterParent>;
static bool Create(ContentParent* aContentParent,
@@ -119,16 +121,17 @@ private:
void DoSendData(Data&& aData);
nsresult EmitStopRequest(nsresult aStatusCode);
virtual void ActorDestroy(ActorDestroyReason aWhy) override;
void Broken();
+ void FinishDisconnect();
void
CheckResult(bool aResult)
{
if (NS_WARN_IF(!aResult)) {
Broken();
}
}
@@ -160,16 +163,17 @@ private:
void RunOnActorThread(const char* aName, Function&& aFunc);
template<typename Function>
void RunOnIOThread(const char* aName, Function&& aFunc);
void RunOnIOThread(already_AddRefed<Runnable>);
nsCOMPtr<nsIChannel> mChannel;
+ nsCOMPtr<nsILoadGroup> mLoadGroup;
nsCOMPtr<nsIStreamListener> mOrigListener;
nsCOMPtr<nsIEventTarget> mMainThread;
nsCOMPtr<nsIEventTarget> mIOThread;
RefPtr<net::ChannelEventQueue> mQueue;
Mutex mBufferMutex;