Commit 53608929 authored by Sean Topping's avatar Sean Topping Committed by Commit Bot

[Chromecast] Allow immediate deletion of expiring CastWebViews

We prefer to eagerly delete all expiring CastWebViews, to prevent UAF
errors if a CastWebView is attempting to use deleted browser objects
very late in the browser lifecycle.

Bug: internal b/150630037
Test: Build/run cast_shell
Change-Id: I9b301c7e76157bb60176dca2588f4bfdff0545c0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2295937Reviewed-by: default avatarYuchen Liu <yucliu@chromium.org>
Commit-Queue: Sean Topping <seantopping@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797858}
parent e77ebb6a
......@@ -65,10 +65,9 @@ CastWebView::Scoped CastWebService::CreateWebView(
const GURL& initial_url) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
auto web_view = web_view_factory_->CreateWebView(params, this, initial_url);
CastWebView::Scoped scoped(web_view.get(), [this](CastWebView* web_view) {
CastWebView::Scoped scoped(web_view.release(), [this](CastWebView* web_view) {
OwnerDestroyed(web_view);
});
web_views_.insert(std::move(web_view));
return scoped;
}
......@@ -111,6 +110,13 @@ void CastWebService::RegisterWebUiClient(
new CastWebUiControllerFactory(std::move(client), hosts));
}
void CastWebService::DeleteExpiringWebViews() {
DCHECK(!immediately_delete_webviews_);
// We don't want to delay webview deletion after this point.
immediately_delete_webviews_ = true;
expiring_web_views_.clear();
}
void CastWebService::OwnerDestroyed(CastWebView* web_view) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
content::WebContents* web_contents = web_view->web_contents();
......@@ -123,13 +129,14 @@ void CastWebService::OwnerDestroyed(CastWebView* web_view) {
->Suspend(content::MediaSession::SuspendType::kSystem);
}
auto delay = web_view->shutdown_delay();
if (delay <= base::TimeDelta()) {
if (delay <= base::TimeDelta() || immediately_delete_webviews_) {
LOG(INFO) << "Immediately deleting CastWebView for " << url;
DeleteWebView(web_view);
delete web_view;
return;
}
LOG(INFO) << "Deleting CastWebView for " << url << " in "
<< delay.InMilliseconds() << " milliseconds.";
expiring_web_views_.emplace(web_view);
task_runner_->PostDelayedTask(
FROM_HERE,
base::BindOnce(&CastWebService::DeleteWebView, weak_ptr_, web_view),
......@@ -138,7 +145,7 @@ void CastWebService::OwnerDestroyed(CastWebView* web_view) {
void CastWebService::DeleteWebView(CastWebView* web_view) {
LOG(INFO) << "Deleting CastWebView.";
base::EraseIf(web_views_,
base::EraseIf(expiring_web_views_,
[web_view](const std::unique_ptr<CastWebView>& ptr) {
return ptr.get() == web_view;
});
......
......@@ -66,6 +66,10 @@ class CastWebService : public mojom::CastWebService {
void RegisterWebUiClient(mojo::PendingRemote<mojom::WebUiClient> client,
const std::vector<std::string>& hosts) override;
// Immediately deletes all owned CastWebViews. This should happen before
// CastWebService is deleted, to prevent UAF of shared browser objects.
void DeleteExpiringWebViews();
private:
void OwnerDestroyed(CastWebView* web_view);
void DeleteWebView(CastWebView* web_view);
......@@ -73,9 +77,14 @@ class CastWebService : public mojom::CastWebService {
content::BrowserContext* const browser_context_;
CastWebViewFactory* const web_view_factory_;
CastWindowManager* const window_manager_;
base::flat_set<std::unique_ptr<CastWebView>> web_views_;
// If a CastWebView is marked for delayed deletion, it is temporarily held
// here for a short time after the original owner releases the CastWebView.
// After the desired shutdown time elapses, the CastWebView is deleted.
base::flat_set<std::unique_ptr<CastWebView>> expiring_web_views_;
const std::unique_ptr<LRURendererCache> overlay_renderer_cache_;
bool immediately_delete_webviews_ = false;
const scoped_refptr<base::SequencedTaskRunner> task_runner_;
base::WeakPtr<CastWebService> weak_ptr_;
......
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