Commit 44f6474d authored by kinuko@chromium.org's avatar kinuko@chromium.org

Fix leak in ServiceWorkerVersionTest.RepeatedlyObserveStatusChanges

In r262636 I deleted the Shutdown() methods from SWRegistration and
SWVersion, but it caused a leak in RepeatedlyObserveStatusChanges.

In the previous code the status change callback (ObserveStatusChanges)
holds a ref to ServiceWorkerVersion, which was ok when we had Shutdown()
(as we could explicitly clear the status change callback there), but
now we don't, so we have cyclic reference there.

I just changed make_scoped_refptr(version) to base::Unretained(version)
when we pass the version to the callback, so that the callback no longer
holds a ref to the Version.

BUG=361504
TEST=ServiceWorkerVersionTest.RepeatedlyObserveStatusChanges with asan/lsan

Review URL: https://codereview.chromium.org/230373003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@262933 0039d316-1c4b-4281-b951-d872f2087c98
parent 17db8dd5
...@@ -182,7 +182,6 @@ ServiceWorkerVersion::ServiceWorkerVersion( ...@@ -182,7 +182,6 @@ ServiceWorkerVersion::ServiceWorkerVersion(
} }
ServiceWorkerVersion::~ServiceWorkerVersion() { ServiceWorkerVersion::~ServiceWorkerVersion() {
status_change_callbacks_.clear();
if (embedded_worker_) { if (embedded_worker_) {
embedded_worker_->RemoveObserver(this); embedded_worker_->RemoveObserver(this);
embedded_worker_.reset(); embedded_worker_.reset();
......
...@@ -94,7 +94,7 @@ void ObserveStatusChanges(ServiceWorkerVersion* version, ...@@ -94,7 +94,7 @@ void ObserveStatusChanges(ServiceWorkerVersion* version,
std::vector<ServiceWorkerVersion::Status>* statuses) { std::vector<ServiceWorkerVersion::Status>* statuses) {
statuses->push_back(version->status()); statuses->push_back(version->status());
version->RegisterStatusChangeCallback( version->RegisterStatusChangeCallback(
base::Bind(&ObserveStatusChanges, make_scoped_refptr(version), statuses)); base::Bind(&ObserveStatusChanges, base::Unretained(version), statuses));
} }
} // namespace } // namespace
...@@ -300,7 +300,7 @@ TEST_F(ServiceWorkerVersionTest, ActivateAndWaitCompletion) { ...@@ -300,7 +300,7 @@ TEST_F(ServiceWorkerVersionTest, ActivateAndWaitCompletion) {
EXPECT_EQ(ServiceWorkerVersion::ACTIVE, version_->status()); EXPECT_EQ(ServiceWorkerVersion::ACTIVE, version_->status());
} }
TEST_F(ServiceWorkerVersionTest, DISABLED_RepeatedlyObserveStatusChanges) { TEST_F(ServiceWorkerVersionTest, RepeatedlyObserveStatusChanges) {
EXPECT_EQ(ServiceWorkerVersion::NEW, version_->status()); EXPECT_EQ(ServiceWorkerVersion::NEW, version_->status());
// Repeatedly observe status changes (the callback re-registers itself). // Repeatedly observe status changes (the callback re-registers itself).
......
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