Commit 42a0127f authored by Rohit Rao's avatar Rohit Rao Committed by Commit Bot

[ios] Fixes a use-after-free in BrowsingDataRemoverImpl.

The BrowsingDataRemoverImpl needs to create a dummy WKWebView in order to remove
cookies. The BrowsingDataRemoverImpl also continues to run removal tasks even
after it or its BrowserState is destroyed. A completion block was incorrectly
referencing an ivar, which turned out to be a garbage pointer in cases where the
BrowserDataRemoverImpl had been destroyed before the completion block ran.

This CL captures the ivar as a local variable before passing it to the block,
avoiding the use-after-free bug. The WKWebView is captured using a strong
reference, because we intentionally want it to stay alive until the block
completes.

BUG=818736

Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: Ic343e0fc1fe48db623245eec4932ceefdf954b70
Reviewed-on: https://chromium-review.googlesource.com/1022031
Commit-Queue: Rohit Rao <rohitrao@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarMohammad Refaat <mrefaat@chromium.org>
Reviewed-by: default avatarSylvain Defresne <sdefresne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552450}
parent f5d457e3
...@@ -600,19 +600,16 @@ void BrowsingDataRemoverImpl::RemoveDataFromWKWebsiteDataStore( ...@@ -600,19 +600,16 @@ void BrowsingDataRemoverImpl::RemoveDataFromWKWebsiteDataStore(
return; return;
} }
// Blocks don't play well with move-only objects, so wrap the closure as base::WeakPtr<BrowsingDataRemoverImpl> weak_ptr = GetWeakPtr();
// a repeating closure (this is better than recreating the same API with __block base::OnceClosure closure = CreatePendingTaskCompletionClosure();
// blocks).
base::RepeatingClosure closure =
AdaptCallbackForRepeating(CreatePendingTaskCompletionClosure());
ProceduralBlock completion_block = ^{ ProceduralBlock completion_block = ^{
dummy_web_view_ = nil; if (BrowsingDataRemoverImpl* strong_ptr = weak_ptr.get())
closure.Run(); strong_ptr->dummy_web_view_ = nil;
std::move(closure).Run();
}; };
if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_VISITED_LINKS)) { if (IsRemoveDataMaskSet(mask, BrowsingDataRemoveMask::REMOVE_VISITED_LINKS)) {
ProceduralBlock previous_completion_block = completion_block; ProceduralBlock previous_completion_block = completion_block;
base::WeakPtr<BrowsingDataRemoverImpl> weak_ptr = GetWeakPtr();
// TODO(crbug.com/557963): Purging the WKProcessPool is a workaround for // TODO(crbug.com/557963): Purging the WKProcessPool is a workaround for
// the fact that there is no public API to clear visited links in // the fact that there is no public API to clear visited links in
......
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