Commit ba3018fa authored by Scott Violet's avatar Scott Violet Committed by Commit Bot

android: fixs access to BookmarkModel from DataProvider

DataProvider::Query() is called from a background thread. BookmarkModel
isn't thread safe, and shouldn't be used from a background thread. This
code should go through bookmarks::ModelLoader and
bookmarks::HistoryBookmarkModel, which are thread safe.

BUG=1032172
TEST=none

Change-Id: I97dd97daed79d64e7ad28b3aa37bfadc0b7d9899
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1961216Reviewed-by: default avatarDonn Denman <donnd@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#723661}
parent e9ca70c2
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "chrome/browser/history/history_service_factory.h" #include "chrome/browser/history/history_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/history_bookmark_model.h"
#include "components/bookmarks/browser/model_loader.h" #include "components/bookmarks/browser/model_loader.h"
#include "components/bookmarks/browser/url_and_title.h" #include "components/bookmarks/browser/url_and_title.h"
#include "components/history/core/browser/history_db_task.h" #include "components/history/core/browser/history_db_task.h"
...@@ -102,11 +103,11 @@ namespace history_report { ...@@ -102,11 +103,11 @@ namespace history_report {
DataProvider::DataProvider(Profile* profile, DataProvider::DataProvider(Profile* profile,
DeltaFileService* delta_file_service, DeltaFileService* delta_file_service,
BookmarkModel* bookmark_model) BookmarkModel* bookmark_model)
: bookmark_model_(bookmark_model), : history_service_(HistoryServiceFactory::GetForProfile(
delta_file_service_(delta_file_service) { profile,
history_service_ = HistoryServiceFactory::GetForProfile( ServiceAccessType::EXPLICIT_ACCESS)),
profile, ServiceAccessType::EXPLICIT_ACCESS); bookmark_model_loader_(bookmark_model->model_loader()),
} delta_file_service_(delta_file_service) {}
DataProvider::~DataProvider() {} DataProvider::~DataProvider() {}
...@@ -127,8 +128,9 @@ std::unique_ptr<std::vector<DeltaFileEntryWithData>> DataProvider::Query( ...@@ -127,8 +128,9 @@ std::unique_ptr<std::vector<DeltaFileEntryWithData>> DataProvider::Query(
base::Unretained(&context), base::Unretained(&context),
base::Unretained(entries.get()))); base::Unretained(entries.get())));
std::vector<UrlAndTitle> bookmarks; std::vector<UrlAndTitle> bookmarks;
bookmark_model_->model_loader()->BlockTillLoaded(); bookmark_model_loader_->BlockTillLoaded();
bookmark_model_->GetBookmarks(&bookmarks); bookmark_model_loader_->history_bookmark_model()->GetBookmarks(
&bookmarks);
BookmarkMap bookmark_map; BookmarkMap bookmark_map;
for (size_t i = 0; i < bookmarks.size(); ++i) { for (size_t i = 0; i < bookmarks.size(); ++i) {
bookmark_map.insert( bookmark_map.insert(
...@@ -187,12 +189,11 @@ void DataProvider::RecreateLog() { ...@@ -187,12 +189,11 @@ void DataProvider::RecreateLog() {
} }
std::vector<UrlAndTitle> bookmarks; std::vector<UrlAndTitle> bookmarks;
bookmark_model_->model_loader()->BlockTillLoaded(); bookmark_model_loader_->BlockTillLoaded();
bookmark_model_->GetBookmarks(&bookmarks); bookmark_model_loader_->history_bookmark_model()->GetBookmarks(&bookmarks);
urls.reserve(urls.size() + bookmarks.size()); urls.reserve(urls.size() + bookmarks.size());
for (size_t i = 0; i < bookmarks.size(); i++) { for (size_t i = 0; i < bookmarks.size(); i++)
urls.push_back(bookmarks[i].url.spec()); urls.push_back(bookmarks[i].url.spec());
}
delta_file_service_->Recreate(urls); delta_file_service_->Recreate(urls);
} }
......
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include <vector> #include <vector>
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h"
#include "base/task/cancelable_task_tracker.h" #include "base/task/cancelable_task_tracker.h"
class Profile; class Profile;
...@@ -21,6 +22,7 @@ class HistoryService; ...@@ -21,6 +22,7 @@ class HistoryService;
namespace bookmarks { namespace bookmarks {
class BookmarkModel; class BookmarkModel;
class ModelLoader;
} }
namespace history_report { namespace history_report {
...@@ -48,7 +50,7 @@ class DataProvider { ...@@ -48,7 +50,7 @@ class DataProvider {
void RecreateLog(); void RecreateLog();
history::HistoryService* history_service_; history::HistoryService* history_service_;
bookmarks::BookmarkModel* bookmark_model_; scoped_refptr<bookmarks::ModelLoader> bookmark_model_loader_;
DeltaFileService* delta_file_service_; DeltaFileService* delta_file_service_;
base::CancelableTaskTracker history_task_tracker_; base::CancelableTaskTracker history_task_tracker_;
......
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