Commit b1dda104 authored by tzik's avatar tzik Committed by Commit Bot

Avoid touching NestedSubscription's ref count before it's fully constructed

NestedSubscription is a ref-counted object, and its first reference used to
be made by base::BindOnce in its constructor. The reference is passed to
another thread, and released when the callback instance is destroyed.

However, if the PostTask failed or the posted task ran soon before the
constructor finished to construct the NestedSubscription instance, the
ref count is decremented to 0, and `new NestedSubscription` may return
a stale pointer.

This CL adds a static constructor to avoid that by splitting the ref-count
related set up out of the constructor.

Bug: 866456
Change-Id: Idf03b31b95b4a7ddee81fdebff78a594e52a62f8
Reviewed-on: https://chromium-review.googlesource.com/1149762Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Commit-Queue: Taiju Tsuiki <tzik@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577933}
parent 0f60eb28
...@@ -49,7 +49,7 @@ class SubscriptionWrapper { ...@@ -49,7 +49,7 @@ class SubscriptionWrapper {
DCHECK(callback_list_.empty()); DCHECK(callback_list_.empty());
nested_subscription_ = nested_subscription_ =
new NestedSubscription(url, name, weak_factory_.GetWeakPtr()); NestedSubscription::Create(url, name, weak_factory_.GetWeakPtr());
return std::make_unique<AwCookieChangeSubscription>( return std::make_unique<AwCookieChangeSubscription>(
callback_list_.Add(std::move(callback))); callback_list_.Add(std::move(callback)));
} }
...@@ -61,21 +61,28 @@ class SubscriptionWrapper { ...@@ -61,21 +61,28 @@ class SubscriptionWrapper {
class NestedSubscription class NestedSubscription
: public base::RefCountedDeleteOnSequence<NestedSubscription> { : public base::RefCountedDeleteOnSequence<NestedSubscription> {
public: public:
NestedSubscription(const GURL& url, static scoped_refptr<NestedSubscription> Create(
const std::string& name, const GURL& url,
base::WeakPtr<SubscriptionWrapper> subscription_wrapper) const std::string& name,
: base::RefCountedDeleteOnSequence<NestedSubscription>( base::WeakPtr<SubscriptionWrapper> subscription_wrapper) {
GetCookieStoreTaskRunner()), auto subscription = base::WrapRefCounted(
subscription_wrapper_(subscription_wrapper), new NestedSubscription(std::move(subscription_wrapper)));
client_task_runner_(base::ThreadTaskRunnerHandle::Get()) { PostTaskToCookieStoreTaskRunner(base::BindOnce(
PostTaskToCookieStoreTaskRunner( &NestedSubscription::Subscribe, subscription, url, name));
base::BindOnce(&NestedSubscription::Subscribe, this, url, name)); return subscription;
} }
private: private:
friend class base::RefCountedDeleteOnSequence<NestedSubscription>; friend class base::RefCountedDeleteOnSequence<NestedSubscription>;
friend class base::DeleteHelper<NestedSubscription>; friend class base::DeleteHelper<NestedSubscription>;
explicit NestedSubscription(
base::WeakPtr<SubscriptionWrapper> subscription_wrapper)
: base::RefCountedDeleteOnSequence<NestedSubscription>(
GetCookieStoreTaskRunner()),
subscription_wrapper_(subscription_wrapper),
client_task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
~NestedSubscription() {} ~NestedSubscription() {}
void Subscribe(const GURL& url, const std::string& name) { void Subscribe(const GURL& url, const std::string& name) {
......
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