Fix issues with request callbacks during browser shutdown (see issue #2622).

The behavior has changed as follows with NetworkService enabled:
- All pending and in-progress requests will now be aborted when the CEF context
  or associated browser is destroyed. The OnResourceLoadComplete callback will
  now also be called in this case for in-progress requests that have a handler.
- The CefResourceHandler::Cancel method will now always be called when resource
  handling is complete, irrespective of whether handling completed successfully.
- Request callbacks that arrive after the OnBeforeClose callback for the
  associated browser (which may happen for in-progress requests that are aborted
  on browser destruction) will now always have a non-nullptr CefBrowser
  parameter.
- Allow empty parameters to CefRequest and CefResponse methods where it makes
  sense (e.g. resetting default response state, or clearing a referrer value).
- Fixed a reference loop that was keeping CefResourceHandler objects from being
  destroyed if they were holding a callback reference (from ProcessRequest,
  ReadResponse, etc.) during CEF context or associated browser destruction.
- Fixed an issue where the main frame was not detached on browser destruction
  which could cause a crash due to RFH use-after-free (see issue #2498).

To test: All unit tests pass as expected.
This commit is contained in:
Marshall Greenblatt
2019-05-31 12:50:12 +03:00
parent fd80e5c653
commit fa5268fa2d
21 changed files with 1458 additions and 369 deletions

View File

@@ -34,12 +34,16 @@ using OnInputStreamOpenedCallback =
class OpenInputStreamWrapper
: public base::RefCountedThreadSafe<OpenInputStreamWrapper> {
public:
static void Open(
static base::OnceClosure Open(
std::unique_ptr<StreamReaderURLLoader::Delegate> delegate,
scoped_refptr<base::SequencedTaskRunner> work_thread_task_runner,
const RequestId& request_id,
const network::ResourceRequest& request,
OnInputStreamOpenedCallback callback) {
OnInputStreamOpenedCallback callback) WARN_UNUSED_RESULT {
scoped_refptr<OpenInputStreamWrapper> wrapper = new OpenInputStreamWrapper(
std::move(delegate), base::ThreadTaskRunnerHandle::Get(),
std::move(callback));
work_thread_task_runner->PostTask(
FROM_HERE,
base::BindOnce(OpenInputStreamWrapper::OpenOnWorkThread,
@@ -47,8 +51,9 @@ class OpenInputStreamWrapper
// while the callback is executing on the background
// thread. The delegate will be "returned" to the loader
// once the InputStream open attempt is completed.
std::move(delegate), base::ThreadTaskRunnerHandle::Get(),
request_id, request, std::move(callback)));
wrapper, request_id, request));
return wrapper->GetCancelCallback();
}
private:
@@ -63,20 +68,26 @@ class OpenInputStreamWrapper
callback_(std::move(callback)) {}
virtual ~OpenInputStreamWrapper() {}
static void OpenOnWorkThread(
std::unique_ptr<StreamReaderURLLoader::Delegate> delegate,
scoped_refptr<base::SingleThreadTaskRunner> job_thread_task_runner,
const RequestId& request_id,
const network::ResourceRequest& request,
OnInputStreamOpenedCallback callback) {
static void OpenOnWorkThread(scoped_refptr<OpenInputStreamWrapper> wrapper,
const RequestId& request_id,
const network::ResourceRequest& request) {
DCHECK(!CEF_CURRENTLY_ON_IOT());
DCHECK(!CEF_CURRENTLY_ON_UIT());
scoped_refptr<OpenInputStreamWrapper> wrapper = new OpenInputStreamWrapper(
std::move(delegate), job_thread_task_runner, std::move(callback));
wrapper->Open(request_id, request);
}
base::OnceClosure GetCancelCallback() {
return base::BindOnce(&OpenInputStreamWrapper::Cancel,
base::WrapRefCounted(this));
}
void Cancel() {
DCHECK(job_thread_task_runner_->RunsTasksInCurrentSequence());
delegate_.reset();
callback_.Reset();
}
void Open(const RequestId& request_id,
const network::ResourceRequest& request) {
if (!delegate_->OpenInputStream(
@@ -96,7 +107,7 @@ class OpenInputStreamWrapper
return;
}
DCHECK(!callback_.is_null());
// May be null if we were canceled.
if (callback_.is_null())
return;
@@ -470,7 +481,12 @@ StreamReaderURLLoader::StreamReaderURLLoader(
base::CreateSequencedTaskRunnerWithTraits({base::MayBlock()});
}
StreamReaderURLLoader::~StreamReaderURLLoader() {}
StreamReaderURLLoader::~StreamReaderURLLoader() {
if (open_cancel_callback_) {
// Release the Delegate held by OpenInputStreamWrapper.
std::move(open_cancel_callback_).Run();
}
}
void StreamReaderURLLoader::Start() {
DCHECK(thread_checker_.CalledOnValidThread());
@@ -503,7 +519,7 @@ void StreamReaderURLLoader::ContinueWithRequestHeaders(
request_.headers = *headers;
}
OpenInputStreamWrapper::Open(
open_cancel_callback_ = OpenInputStreamWrapper::Open(
// This is intentional - the loader could be deleted while
// the callback is executing on the background thread. The
// delegate will be "returned" to the loader once the
@@ -536,6 +552,7 @@ void StreamReaderURLLoader::OnInputStreamOpened(
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(returned_delegate);
response_delegate_ = std::move(returned_delegate);
open_cancel_callback_.Reset();
if (!input_stream) {
bool restarted = false;