Skip to content

Commit

Permalink
[SyncFS] Make ExtensionService usage of SyncWorker asynchronous
Browse files Browse the repository at this point in the history 
As a preparation of thread migration from UI to a worker,
this CL moves the usage of ExtensionService in SyncWorker to UI thread explicitly.

BUG=347425

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@266277 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
tzik@chromium.org committed Apr 25, 2014
1 parent 44a7ef9 commit f03456c
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 44 deletions.
6 changes: 5 additions & 1 deletion chrome/browser/sync_file_system/drive_backend/sync_engine.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,15 @@ void SyncEngine::Initialize(const base::FilePath& base_dir,
new WorkerObserver(base::MessageLoopProxy::current(),
weak_ptr_factory_.GetWeakPtr()));

base::WeakPtr<ExtensionServiceInterface> extension_service_weak_ptr;
if (extension_service_)
extension_service_weak_ptr = extension_service_->AsWeakPtr();

// TODO(peria): Use PostTask on |worker_task_runner_| to call this function.
sync_worker_ = SyncWorker::CreateOnWorker(
base_dir,
worker_observer_.get(),
extension_service_,
extension_service_weak_ptr,
sync_engine_context.Pass(),
env_override);

Expand Down
119 changes: 81 additions & 38 deletions chrome/browser/sync_file_system/drive_backend/sync_worker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
#include "chrome/browser/signin/signin_manager_factory.h"
#include "chrome/browser/sync_file_system/drive_backend/callback_helper.h"
#include "chrome/browser/sync_file_system/drive_backend/conflict_resolver.h"
#include "chrome/browser/sync_file_system/drive_backend/drive_backend_constants.h"
#include "chrome/browser/sync_file_system/drive_backend/list_changes_task.h"
Expand Down Expand Up @@ -55,12 +56,37 @@ namespace {

void EmptyStatusCallback(SyncStatusCode status) {}

void QueryAppStatusOnUIThread(
const base::WeakPtr<ExtensionServiceInterface>& extension_service_ptr,
const std::vector<std::string>* app_ids,
SyncWorker::AppStatusMap* status,
const base::Closure& callback) {
ExtensionServiceInterface* extension_service = extension_service_ptr.get();
if (!extension_service) {
callback.Run();
return;
}

for (std::vector<std::string>::const_iterator itr = app_ids->begin();
itr != app_ids->end(); ++itr) {
const std::string& app_id = *itr;
if (!extension_service->GetInstalledExtension(app_id))
(*status)[app_id] = SyncWorker::APP_STATUS_UNINSTALLED;
else if (!extension_service->IsExtensionEnabled(app_id))
(*status)[app_id] = SyncWorker::APP_STATUS_DISABLED;
else
(*status)[app_id] = SyncWorker::APP_STATUS_ENABLED;
}

callback.Run();
}

} // namespace

scoped_ptr<SyncWorker> SyncWorker::CreateOnWorker(
const base::FilePath& base_dir,
Observer* observer,
ExtensionServiceInterface* extension_service,
const base::WeakPtr<ExtensionServiceInterface>& extension_service,
scoped_ptr<SyncEngineContext> sync_engine_context,
leveldb::Env* env_override) {
scoped_ptr<SyncWorker> sync_worker(
Expand Down Expand Up @@ -355,7 +381,7 @@ void SyncWorker::AddObserver(Observer* observer) {

SyncWorker::SyncWorker(
const base::FilePath& base_dir,
ExtensionServiceInterface* extension_service,
const base::WeakPtr<ExtensionServiceInterface>& extension_service,
scoped_ptr<SyncEngineContext> sync_engine_context,
leveldb::Env* env_override)
: base_dir_(base_dir),
Expand Down Expand Up @@ -433,47 +459,64 @@ void SyncWorker::DidInitialize(SyncEngineInitializer* initializer,
}

void SyncWorker::UpdateRegisteredApp() {
if (extension_service_)
return;
MetadataDatabase* metadata_db = GetMetadataDatabase();
DCHECK(metadata_db);

scoped_ptr<std::vector<std::string> > app_ids(new std::vector<std::string>);
metadata_db->GetRegisteredAppIDs(app_ids.get());

AppStatusMap* app_status = new AppStatusMap;
base::Closure callback =
base::Bind(&SyncWorker::DidQueryAppStatus,
weak_ptr_factory_.GetWeakPtr(),
base::Owned(app_status));

context_->GetUITaskRunner()->PostTask(
FROM_HERE,
base::Bind(&QueryAppStatusOnUIThread,
extension_service_,
base::Owned(app_ids.release()),
app_status,
RelayCallbackToTaskRunner(
context_->GetWorkerTaskRunner(),
FROM_HERE, callback)));
}

void SyncWorker::DidQueryAppStatus(const AppStatusMap* app_status) {
MetadataDatabase* metadata_db = GetMetadataDatabase();
DCHECK(metadata_db);
std::vector<std::string> app_ids;
metadata_db->GetRegisteredAppIDs(&app_ids);

// Update the status of every origin using status from ExtensionService.
for (std::vector<std::string>::const_iterator itr = app_ids.begin();
itr != app_ids.end(); ++itr) {
const std::string& app_id = *itr;
GURL origin =
extensions::Extension::GetBaseURLFromExtensionId(app_id);

// TODO(tzik): Switch |extension_service_| to a wrapper and make this
// call async.
if (!extension_service_->GetInstalledExtension(app_id)) {
// Extension has been uninstalled.
// (At this stage we can't know if it was unpacked extension or not,
// so just purge the remote folder.)
UninstallOrigin(origin,
RemoteFileSyncService::UNINSTALL_AND_PURGE_REMOTE,
base::Bind(&EmptyStatusCallback));
continue;
}
FileTracker tracker;
if (!metadata_db->FindAppRootTracker(app_id, &tracker)) {
// App will register itself on first run.
continue;
}

// TODO(tzik): Switch |extension_service_| to a wrapper and make this
// call async.
bool is_app_enabled = extension_service_->IsExtensionEnabled(app_id);
bool is_app_root_tracker_enabled =
tracker.tracker_kind() == TRACKER_KIND_APP_ROOT;
if (is_app_enabled && !is_app_root_tracker_enabled)
EnableOrigin(origin, base::Bind(&EmptyStatusCallback));
else if (!is_app_enabled && is_app_root_tracker_enabled)
DisableOrigin(origin, base::Bind(&EmptyStatusCallback));
for (AppStatusMap::const_iterator itr = app_status->begin();
itr != app_status->end(); ++itr) {
const std::string& app_id = itr->first;
GURL origin = extensions::Extension::GetBaseURLFromExtensionId(app_id);

if (itr->second == APP_STATUS_UNINSTALLED) {
// Extension has been uninstalled.
// (At this stage we can't know if it was unpacked extension or not,
// so just purge the remote folder.)
UninstallOrigin(origin,
RemoteFileSyncService::UNINSTALL_AND_PURGE_REMOTE,
base::Bind(&EmptyStatusCallback));
continue;
}

FileTracker tracker;
if (!metadata_db->FindAppRootTracker(app_id, &tracker)) {
// App will register itself on first run.
continue;
}

DCHECK(itr->second == APP_STATUS_ENABLED ||
itr->second == APP_STATUS_DISABLED);
bool is_app_enabled = (itr->second == APP_STATUS_ENABLED);
bool is_app_root_tracker_enabled =
(tracker.tracker_kind() == TRACKER_KIND_APP_ROOT);
if (is_app_enabled && !is_app_root_tracker_enabled)
EnableOrigin(origin, base::Bind(&EmptyStatusCallback));
else if (!is_app_enabled && is_app_root_tracker_enabled)
DisableOrigin(origin, base::Bind(&EmptyStatusCallback));
}
}

Expand Down
17 changes: 12 additions & 5 deletions chrome/browser/sync_file_system/drive_backend/sync_worker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ class SyncEngineInitializer;

class SyncWorker : public SyncTaskManager::Client {
public:
enum AppStatus {
APP_STATUS_ENABLED,
APP_STATUS_DISABLED,
APP_STATUS_UNINSTALLED,
};

typedef base::hash_map<std::string, AppStatus> AppStatusMap;

class Observer {
public:
virtual void OnPendingFileListUpdated(int item_count) = 0;
Expand All @@ -67,7 +75,7 @@ class SyncWorker : public SyncTaskManager::Client {
static scoped_ptr<SyncWorker> CreateOnWorker(
const base::FilePath& base_dir,
Observer* observer,
ExtensionServiceInterface* extension_service,
const base::WeakPtr<ExtensionServiceInterface>& extension_service,
scoped_ptr<SyncEngineContext> sync_engine_context,
leveldb::Env* env_override);

Expand Down Expand Up @@ -130,7 +138,7 @@ class SyncWorker : public SyncTaskManager::Client {
friend class SyncEngineTest;

SyncWorker(const base::FilePath& base_dir,
ExtensionServiceInterface* extension_service,
const base::WeakPtr<ExtensionServiceInterface>& extension_service,
scoped_ptr<SyncEngineContext> sync_engine_context,
leveldb::Env* env_override);

Expand All @@ -143,6 +151,7 @@ class SyncWorker : public SyncTaskManager::Client {
void DidInitialize(SyncEngineInitializer* initializer,
SyncStatusCode status);
void UpdateRegisteredApp();
void DidQueryAppStatus(const AppStatusMap* app_status);
void DidProcessRemoteChange(RemoteToLocalSyncer* syncer,
const SyncFileCallback& callback,
SyncStatusCode status);
Expand Down Expand Up @@ -178,9 +187,7 @@ class SyncWorker : public SyncTaskManager::Client {

scoped_ptr<SyncTaskManager> task_manager_;

// TODO(tzik): Add a proxy class for ExtensionServiceInterface to cross
// thread, and hold its instance as WeakPtr here.
ExtensionServiceInterface* extension_service_;
base::WeakPtr<ExtensionServiceInterface> extension_service_;

scoped_ptr<SyncEngineContext> context_;
ObserverList<Observer> observers_;
Expand Down

0 comments on commit f03456c

Please sign in to comment.