Commit 20a36c5b authored by Ehsan Karamad's avatar Ehsan Karamad Committed by Commit Bot

PluginResourceThrottle to use WeakPtr to MimeHandlerViewContainerBase

This is a second attempt at fixing the crash linked in the bug. It
appears that PluginResourceThrottle's lifetime is tied with
WebURLLoaderImpl does not synchronously go away with the container.

When an <embed> element is removed during a resource request,
the MHVCBase (aka WebAssociatedURLLoaderClient in this context) is
destroyed immediately.

The WebURLLoaderImpl, OTH, goes away when the cancel timer on the
ResourceClient is fired.

To avoid any potential UaF, this CL changes the raw pointer refernece
to MHVCBase in PluginResourceThrottle to a WeakPtr. This also serves
as a potential fix for the crashes in the linked bug.

Bug: 878359
Change-Id: If8b21f9366bb57394819946e891f9288028dde2c
Reviewed-on: https://chromium-review.googlesource.com/1230576Reviewed-by: default avatarJames MacLean <wjmaclean@chromium.org>
Reviewed-by: default avatarEhsan Karamad <ekaramad@chromium.org>
Commit-Queue: Ehsan Karamad <ekaramad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592114}
parent c033c2bd
...@@ -125,7 +125,8 @@ base::LazyInstance< ...@@ -125,7 +125,8 @@ base::LazyInstance<
class MimeHandlerViewContainerBase::PluginResourceThrottle class MimeHandlerViewContainerBase::PluginResourceThrottle
: public content::URLLoaderThrottle { : public content::URLLoaderThrottle {
public: public:
explicit PluginResourceThrottle(MimeHandlerViewContainerBase* container) explicit PluginResourceThrottle(
base::WeakPtr<MimeHandlerViewContainerBase> container)
: container_(container) {} : container_(container) {}
~PluginResourceThrottle() override {} ~PluginResourceThrottle() override {}
...@@ -134,6 +135,14 @@ class MimeHandlerViewContainerBase::PluginResourceThrottle ...@@ -134,6 +135,14 @@ class MimeHandlerViewContainerBase::PluginResourceThrottle
void WillProcessResponse(const GURL& response_url, void WillProcessResponse(const GURL& response_url,
network::ResourceResponseHead* response_head, network::ResourceResponseHead* response_head,
bool* defer) override { bool* defer) override {
if (!container_) {
// In the embedder case if the plugin element is removed right after an
// ongoing request is made, MimeHandlerViewContainerBase is destroyed
// synchronously but the WebURLLoaderImpl corresponding to this throttle
// goes away asynchronously when ResourceLoader::CancelTimerFired() is
// called (see https://crbug.com/878359).
return;
}
network::mojom::URLLoaderPtr dummy_new_loader; network::mojom::URLLoaderPtr dummy_new_loader;
mojo::MakeRequest(&dummy_new_loader); mojo::MakeRequest(&dummy_new_loader);
network::mojom::URLLoaderClientPtr new_client; network::mojom::URLLoaderClientPtr new_client;
...@@ -158,7 +167,7 @@ class MimeHandlerViewContainerBase::PluginResourceThrottle ...@@ -158,7 +167,7 @@ class MimeHandlerViewContainerBase::PluginResourceThrottle
container_->SetEmbeddedLoader(std::move(transferrable_loader)); container_->SetEmbeddedLoader(std::move(transferrable_loader));
} }
MimeHandlerViewContainerBase* container_; base::WeakPtr<MimeHandlerViewContainerBase> container_;
DISALLOW_COPY_AND_ASSIGN(PluginResourceThrottle); DISALLOW_COPY_AND_ASSIGN(PluginResourceThrottle);
}; };
...@@ -226,7 +235,7 @@ MimeHandlerViewContainerBase::MaybeCreatePluginThrottle(const GURL& url) { ...@@ -226,7 +235,7 @@ MimeHandlerViewContainerBase::MaybeCreatePluginThrottle(const GURL& url) {
return nullptr; return nullptr;
waiting_to_create_throttle_ = false; waiting_to_create_throttle_ = false;
return std::make_unique<PluginResourceThrottle>(this); return std::make_unique<PluginResourceThrottle>(weak_factory_.GetWeakPtr());
} }
void MimeHandlerViewContainerBase::PostJavaScriptMessage( void MimeHandlerViewContainerBase::PostJavaScriptMessage(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment