Commit 80f52c68 authored by sdefresne's avatar sdefresne Committed by Commit bot

Fix lifetime of EnhancedBookmarkModel & BookmarkImageService

Unregister BookmarkImageService observer from BookmarkModel in the
Shutdown method instead of the destructor to follow the two step
shutdown of the profile.

Fix a typo in the EnhancedBookmarkModel override of KeyedService
Shutdown method (could have been caught by using the OVERRIDE keyword).

BUG=None

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

Cr-Commit-Position: refs/heads/master@{#296009}
parent 6060457a
...@@ -83,13 +83,18 @@ BookmarkImageService::BookmarkImageService( ...@@ -83,13 +83,18 @@ BookmarkImageService::BookmarkImageService(
BookmarkImageService::~BookmarkImageService() { BookmarkImageService::~BookmarkImageService() {
DCHECK(CalledOnValidThread()); DCHECK(CalledOnValidThread());
enhanced_bookmark_model_->bookmark_model()->RemoveObserver(this);
pool_->PostNamedSequencedWorkerTask( pool_->PostNamedSequencedWorkerTask(
kSequenceToken, kSequenceToken,
FROM_HERE, FROM_HERE,
base::Bind(&DeleteImageStore, store_.release())); base::Bind(&DeleteImageStore, store_.release()));
} }
void BookmarkImageService::Shutdown() {
DCHECK(CalledOnValidThread());
enhanced_bookmark_model_->bookmark_model()->RemoveObserver(this);
enhanced_bookmark_model_ = NULL;
}
void BookmarkImageService::SalientImageForUrl(const GURL& page_url, void BookmarkImageService::SalientImageForUrl(const GURL& page_url,
Callback callback) { Callback callback) {
DCHECK(CalledOnValidThread()); DCHECK(CalledOnValidThread());
......
...@@ -38,6 +38,9 @@ class BookmarkImageService : public KeyedService, ...@@ -38,6 +38,9 @@ class BookmarkImageService : public KeyedService,
typedef base::Callback<void(const gfx::Image&, const GURL& url)> Callback; typedef base::Callback<void(const gfx::Image&, const GURL& url)> Callback;
// KeyedService:
virtual void Shutdown() OVERRIDE;
// Returns a salient image for a URL. This may trigger a network request for // Returns a salient image for a URL. This may trigger a network request for
// the image if the image was not retrieved before and if a bookmark node has // the image if the image was not retrieved before and if a bookmark node has
// a URL for this salient image available. The image (which may be empty) is // a URL for this salient image available. The image (which may be empty) is
......
...@@ -93,7 +93,7 @@ EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model, ...@@ -93,7 +93,7 @@ EnhancedBookmarkModel::EnhancedBookmarkModel(BookmarkModel* bookmark_model,
EnhancedBookmarkModel::~EnhancedBookmarkModel() { EnhancedBookmarkModel::~EnhancedBookmarkModel() {
} }
void EnhancedBookmarkModel::ShutDown() { void EnhancedBookmarkModel::Shutdown() {
FOR_EACH_OBSERVER(EnhancedBookmarkModelObserver, FOR_EACH_OBSERVER(EnhancedBookmarkModelObserver,
observers_, observers_,
EnhancedBookmarkModelShuttingDown()); EnhancedBookmarkModelShuttingDown());
......
...@@ -38,7 +38,7 @@ class EnhancedBookmarkModel : public KeyedService, ...@@ -38,7 +38,7 @@ class EnhancedBookmarkModel : public KeyedService,
const std::string& version); const std::string& version);
virtual ~EnhancedBookmarkModel(); virtual ~EnhancedBookmarkModel();
virtual void ShutDown(); virtual void Shutdown() OVERRIDE;
void AddObserver(EnhancedBookmarkModelObserver* observer); void AddObserver(EnhancedBookmarkModelObserver* observer);
void RemoveObserver(EnhancedBookmarkModelObserver* observer); void RemoveObserver(EnhancedBookmarkModelObserver* observer);
......
...@@ -51,7 +51,7 @@ class EnhancedBookmarkModelTest ...@@ -51,7 +51,7 @@ class EnhancedBookmarkModelTest
virtual void TearDown() OVERRIDE { virtual void TearDown() OVERRIDE {
if (model_) if (model_)
model_->ShutDown(); model_->Shutdown();
model_.reset(); model_.reset();
bookmark_model_.reset(); bookmark_model_.reset();
bookmark_client_.reset(); bookmark_client_.reset();
...@@ -447,7 +447,7 @@ TEST_F(EnhancedBookmarkModelTest, TestVersionField) { ...@@ -447,7 +447,7 @@ TEST_F(EnhancedBookmarkModelTest, TestVersionField) {
// Verifies that duplicate nodes are reset when the model is created. // Verifies that duplicate nodes are reset when the model is created.
TEST_F(EnhancedBookmarkModelTest, ResetDuplicateNodesOnInitialization) { TEST_F(EnhancedBookmarkModelTest, ResetDuplicateNodesOnInitialization) {
model_->ShutDown(); model_->Shutdown();
const BookmarkNode* parent = bookmark_model_->other_node(); const BookmarkNode* parent = bookmark_model_->other_node();
const BookmarkNode* node1 = bookmark_model_->AddURL( const BookmarkNode* node1 = bookmark_model_->AddURL(
...@@ -563,7 +563,7 @@ TEST_F(EnhancedBookmarkModelTest, SetMultipleMetaInfo) { ...@@ -563,7 +563,7 @@ TEST_F(EnhancedBookmarkModelTest, SetMultipleMetaInfo) {
TEST_F(EnhancedBookmarkModelTest, ObserverShuttingDownEvent) { TEST_F(EnhancedBookmarkModelTest, ObserverShuttingDownEvent) {
EXPECT_EQ(0, shutting_down_calls_); EXPECT_EQ(0, shutting_down_calls_);
model_->ShutDown(); model_->Shutdown();
EXPECT_EQ(1, shutting_down_calls_); EXPECT_EQ(1, shutting_down_calls_);
model_.reset(); model_.reset();
} }
...@@ -645,7 +645,7 @@ TEST_F(EnhancedBookmarkModelTest, ShutDownWhileResetDuplicationScheduled) { ...@@ -645,7 +645,7 @@ TEST_F(EnhancedBookmarkModelTest, ShutDownWhileResetDuplicationScheduled) {
const BookmarkNode* node2 = AddBookmark(); const BookmarkNode* node2 = AddBookmark();
bookmark_model_->SetNodeMetaInfo(node1, "stars.id", "c_1"); bookmark_model_->SetNodeMetaInfo(node1, "stars.id", "c_1");
bookmark_model_->SetNodeMetaInfo(node2, "stars.id", "c_1"); bookmark_model_->SetNodeMetaInfo(node2, "stars.id", "c_1");
model_->ShutDown(); model_->Shutdown();
model_.reset(); model_.reset();
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
} }
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