fix: make StreamSubscriber ref counted (#17221)

It is owned by URLRequestStreamJob on the IO thread once request starts,
but if the ownership was abondoned while transfering it to IO thread
which is possible when a request is aborted, then we need to make sure
its destroyed on the right thread to avoid lock in v8.
This commit is contained in:
Robo 2019-03-07 20:50:03 +05:30 committed by GitHub
parent b575631bb0
commit d4d6b9862f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 24 deletions

View file

@ -17,6 +17,7 @@
#include "base/strings/string_number_conversions.h"
#include "base/strings/string_util.h"
#include "base/task/post_task.h"
#include "base/threading/thread_task_runner_handle.h"
#include "base/time/time.h"
#include "content/public/browser/browser_task_traits.h"
#include "native_mate/dictionary.h"
@ -84,14 +85,14 @@ void BeforeStartInUI(base::WeakPtr<URLRequestStreamJob> job,
return;
}
auto subscriber = std::make_unique<mate::StreamSubscriber>(
args->isolate(), data.GetHandle(), job);
auto subscriber = base::MakeRefCounted<mate::StreamSubscriber>(
args->isolate(), data.GetHandle(), job,
base::ThreadTaskRunnerHandle::Get());
base::PostTaskWithTraits(
FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&URLRequestStreamJob::StartAsync, job,
std::move(subscriber), base::RetainedRef(response_headers),
ended, error));
base::BindOnce(&URLRequestStreamJob::StartAsync, job, subscriber,
base::RetainedRef(response_headers), ended, error));
}
} // namespace
@ -106,10 +107,7 @@ URLRequestStreamJob::URLRequestStreamJob(net::URLRequest* request,
weak_factory_(this) {}
URLRequestStreamJob::~URLRequestStreamJob() {
if (subscriber_) {
content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE,
std::move(subscriber_));
}
DCHECK(!subscriber_ || subscriber_->HasOneRef());
}
void URLRequestStreamJob::Start() {
@ -123,7 +121,7 @@ void URLRequestStreamJob::Start() {
}
void URLRequestStreamJob::StartAsync(
std::unique_ptr<mate::StreamSubscriber> subscriber,
scoped_refptr<mate::StreamSubscriber> subscriber,
scoped_refptr<net::HttpResponseHeaders> response_headers,
bool ended,
int error) {
@ -135,7 +133,7 @@ void URLRequestStreamJob::StartAsync(
ended_ = ended;
response_headers_ = response_headers;
subscriber_ = std::move(subscriber);
subscriber_ = subscriber;
request_start_time_ = base::TimeTicks::Now();
NotifyHeadersComplete();
}
@ -194,12 +192,14 @@ int URLRequestStreamJob::ReadRawData(net::IOBuffer* dest, int dest_size) {
}
void URLRequestStreamJob::DoneReading() {
content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE,
std::move(subscriber_));
write_buffer_.clear();
}
void URLRequestStreamJob::DoneReadingRedirectResponse() {
if (subscriber_) {
DCHECK(subscriber_->HasAtLeastOneRef());
subscriber_ = nullptr;
}
DoneReading();
}