Skip to content

Commit

Permalink
Fixed Team Drives data to be fetched on Delta Feed Fetch
Browse files Browse the repository at this point in the history 
Previously, the files app wouldn't check for changes to the flag on startup.
This patch introduces a check in change_list_loader.cc that forces a fetch
of the team drives listing on both a delta and full fetch.

files app again. Team drives should appear.

Test: Open the files app, change the flag, restart chrome, then open the
TBR: fukino
Bug: 723141
Change-Id: I8310b39b916609181de332622def975b8f02a8e2
Reviewed-on: https://chromium-review.googlesource.com/979375
Commit-Queue: Sasha Morrissey <sashab@chromium.org>
Reviewed-by: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550515}
  • Loading branch information
Sasha Morrissey authored and Commit Bot committed Apr 13, 2018
1 parent f591313 commit 02a37b8
Show file tree
Hide file tree
Showing 2 changed files with 119 additions and 50 deletions.
153 changes: 106 additions & 47 deletions components/drive/chromeos/change_list_loader.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,32 +42,20 @@ class ChangeListLoader::FeedFetcher {

namespace {

// Fetches all the (currently available) resource entries from the server.
class FullFeedFetcher : public ChangeListLoader::FeedFetcher {
// Fetches the list of team drives from the server.
class TeamDriveListFetcher : public ChangeListLoader::FeedFetcher {
public:
FullFeedFetcher(
JobScheduler* scheduler,
google_apis::TeamDrivesIntegrationStatus team_drives_integration)
: scheduler_(scheduler),
is_team_drive_enabled_(team_drives_integration ==
google_apis::TEAM_DRIVES_INTEGRATION_ENABLED),
weak_ptr_factory_(this) {}
TeamDriveListFetcher(JobScheduler* scheduler)
: scheduler_(scheduler), weak_ptr_factory_(this) {}

~FullFeedFetcher() override {}
~TeamDriveListFetcher() override {}

void Run(const FeedFetcherCallback& callback) override {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!callback.is_null());

if (!is_team_drive_enabled_) {
StartFetchFileList(callback);
return;
}

// Fetch Team Drive before File list, so that files can be stored under
// root directories of each Team Drive like /team_drive/My Team Drive/.
scheduler_->GetAllTeamDriveList(
base::Bind(&FullFeedFetcher::OnTeamDriveListFetched,
base::Bind(&TeamDriveListFetcher::OnTeamDriveListFetched,
weak_ptr_factory_.GetWeakPtr(), callback));
}

Expand All @@ -76,20 +64,51 @@ class FullFeedFetcher : public ChangeListLoader::FeedFetcher {
const FeedFetcherCallback& callback,
google_apis::DriveApiErrorCode status,
std::unique_ptr<google_apis::TeamDriveList> team_drives) {
DCHECK(is_team_drive_enabled_);
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!callback.is_null());

FileError error = GDataToFileError(status);
if (error != FILE_ERROR_OK) {
callback.Run(error, std::vector<std::unique_ptr<ChangeList>>());
return;
}

change_lists_.push_back(std::make_unique<ChangeList>(*team_drives));

// Fetch more drives, if there are more.
if (!team_drives->next_page_token().empty()) {
scheduler_->GetRemainingTeamDriveList(
team_drives->next_page_token(),
base::Bind(&FullFeedFetcher::OnTeamDriveListFetched,
base::Bind(&TeamDriveListFetcher::OnTeamDriveListFetched,
weak_ptr_factory_.GetWeakPtr(), callback));
return;
}
StartFetchFileList(callback);

// Note: The fetcher is managed by ChangeListLoader, and the instance
// will be deleted in the callback. Do not touch the fields after this
// invocation.
callback.Run(FILE_ERROR_OK, std::move(change_lists_));
}

void StartFetchFileList(const FeedFetcherCallback& callback) {
JobScheduler* scheduler_;
std::vector<std::unique_ptr<ChangeList>> change_lists_;
base::ThreadChecker thread_checker_;
base::WeakPtrFactory<TeamDriveListFetcher> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(TeamDriveListFetcher);
};

// Fetches all the (currently available) resource entries from the server.
class FullFeedFetcher : public ChangeListLoader::FeedFetcher {
public:
FullFeedFetcher(JobScheduler* scheduler)
: scheduler_(scheduler), weak_ptr_factory_(this) {}

~FullFeedFetcher() override {}

void Run(const FeedFetcherCallback& callback) override {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!callback.is_null());

// Remember the time stamp for usage stats.
start_time_ = base::TimeTicks::Now();
// This is full resource list fetch.
Expand All @@ -101,6 +120,7 @@ class FullFeedFetcher : public ChangeListLoader::FeedFetcher {
weak_ptr_factory_.GetWeakPtr(), callback));
}

private:
void OnFileListFetched(const FeedFetcherCallback& callback,
google_apis::DriveApiErrorCode status,
std::unique_ptr<google_apis::FileList> file_list) {
Expand Down Expand Up @@ -138,7 +158,6 @@ class FullFeedFetcher : public ChangeListLoader::FeedFetcher {
std::vector<std::unique_ptr<ChangeList>> change_lists_;
base::TimeTicks start_time_;
base::ThreadChecker thread_checker_;
bool is_team_drive_enabled_;
base::WeakPtrFactory<FullFeedFetcher> weak_ptr_factory_;
DISALLOW_COPY_AND_ASSIGN(FullFeedFetcher);
};
Expand Down Expand Up @@ -461,6 +480,7 @@ void ChangeListLoader::LoadAfterGetAboutResource(
google_apis::DriveApiErrorCode status,
std::unique_ptr<google_apis::AboutResource> about_resource) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!change_feed_fetcher_);

FileError error = GDataToFileError(status);
if (error != FILE_ERROR_OK) {
Expand All @@ -470,20 +490,21 @@ void ChangeListLoader::LoadAfterGetAboutResource(

DCHECK(about_resource);

int64_t remote_changestamp = about_resource->largest_change_id();
int64_t start_changestamp = local_changestamp > 0 ? local_changestamp + 1 : 0;
if (local_changestamp >= remote_changestamp) {
if (local_changestamp > remote_changestamp) {
LOG(WARNING) << "Local resource metadata is fresher than server, "
<< "local = " << local_changestamp
<< ", server = " << remote_changestamp;
}
// Fetch Team Drive before File list, so that files can be stored under root
// directories of each Team Drive like /team_drive/My Team Drive/.
if (google_apis::GetTeamDrivesIntegrationSwitch() ==
google_apis::TEAM_DRIVES_INTEGRATION_ENABLED) {
change_feed_fetcher_.reset(new TeamDriveListFetcher(scheduler_));

// No changes detected, tell the client that the loading was successful.
OnChangeListLoadComplete(FILE_ERROR_OK);
change_feed_fetcher_->Run(
base::Bind(&ChangeListLoader::LoadChangeListFromServer,
weak_ptr_factory_.GetWeakPtr(),
base::Passed(&about_resource), local_changestamp));
} else {
// Start loading the change list.
LoadChangeListFromServer(start_changestamp);
// If there are no team drives listings, the changelist starts as empty.
LoadChangeListFromServer(std::move(about_resource), local_changestamp,
FILE_ERROR_OK,
std::vector<std::unique_ptr<ChangeList>>());
}
}

Expand Down Expand Up @@ -526,35 +547,63 @@ void ChangeListLoader::OnAboutResourceUpdated(
base::Int64ToString(resource->largest_change_id()).c_str());
}

void ChangeListLoader::LoadChangeListFromServer(int64_t start_changestamp) {
void ChangeListLoader::LoadChangeListFromServer(
std::unique_ptr<google_apis::AboutResource> about_resource,
int64_t local_changestamp,
FileError error,
std::vector<std::unique_ptr<ChangeList>> team_drives_change_lists) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK(!change_feed_fetcher_);
DCHECK(about_resource_loader_->cached_about_resource());
DCHECK(about_resource);

bool is_delta_update = start_changestamp != 0;
if (error != FILE_ERROR_OK) {
OnChangeListLoadComplete(error);
return;
}

int64_t remote_changestamp = about_resource->largest_change_id();
int64_t start_changestamp = local_changestamp > 0 ? local_changestamp + 1 : 0;
if (local_changestamp >= remote_changestamp) {
if (local_changestamp > remote_changestamp) {
LOG(WARNING) << "Local resource metadata is fresher than server, "
<< "local = " << local_changestamp
<< ", server = " << remote_changestamp;
}

// If there are team drives change lists, update those without running a
// feed fetcher.
if (!team_drives_change_lists.empty()) {
LoadChangeListFromServerAfterLoadChangeList(
std::move(about_resource), true, std::move(team_drives_change_lists),
FILE_ERROR_OK, std::vector<std::unique_ptr<ChangeList>>());
return;
}

// No changes detected, tell the client that the loading was successful.
OnChangeListLoadComplete(FILE_ERROR_OK);
return;
}

// Set up feed fetcher.
bool is_delta_update = start_changestamp != 0;
if (is_delta_update) {
change_feed_fetcher_.reset(
new DeltaFeedFetcher(scheduler_, start_changestamp));
} else {
change_feed_fetcher_.reset(new FullFeedFetcher(
scheduler_, google_apis::GetTeamDrivesIntegrationSwitch()));
change_feed_fetcher_.reset(new FullFeedFetcher(scheduler_));
}

// Make a copy of cached_about_resource_ to remember at which changestamp we
// are fetching change list.
change_feed_fetcher_->Run(
base::Bind(&ChangeListLoader::LoadChangeListFromServerAfterLoadChangeList,
weak_ptr_factory_.GetWeakPtr(),
base::Passed(std::make_unique<google_apis::AboutResource>(
*about_resource_loader_->cached_about_resource())),
is_delta_update));
weak_ptr_factory_.GetWeakPtr(), base::Passed(&about_resource),
is_delta_update, base::Passed(&team_drives_change_lists)));
}

void ChangeListLoader::LoadChangeListFromServerAfterLoadChangeList(
std::unique_ptr<google_apis::AboutResource> about_resource,
bool is_delta_update,
std::vector<std::unique_ptr<ChangeList>> team_drives_change_lists,
FileError error,
std::vector<std::unique_ptr<ChangeList>> change_lists) {
DCHECK(thread_checker_.CalledOnValidThread());
Expand All @@ -568,6 +617,16 @@ void ChangeListLoader::LoadChangeListFromServerAfterLoadChangeList(
return;
}

// Merge the change lists - first team drives, then changes.
std::vector<std::unique_ptr<ChangeList>> merged_change_lists;
merged_change_lists.insert(
merged_change_lists.end(),
std::make_move_iterator(team_drives_change_lists.begin()),
std::make_move_iterator(team_drives_change_lists.end()));
merged_change_lists.insert(merged_change_lists.end(),
std::make_move_iterator(change_lists.begin()),
std::make_move_iterator(change_lists.end()));

ChangeListProcessor* change_list_processor =
new ChangeListProcessor(resource_metadata_, in_shutdown_.get());
// Don't send directory content change notification while performing
Expand All @@ -582,8 +641,8 @@ void ChangeListLoader::LoadChangeListFromServerAfterLoadChangeList(
FROM_HERE,
base::Bind(&ChangeListProcessor::ApplyUserChangeList,
base::Unretained(change_list_processor),
base::Passed(&about_resource), base::Passed(&change_lists),
is_delta_update),
base::Passed(&about_resource),
base::Passed(&merged_change_lists), is_delta_update),
base::Bind(&ChangeListLoader::LoadChangeListFromServerAfterUpdate,
weak_ptr_factory_.GetWeakPtr(),
base::Owned(change_list_processor),
Expand Down
16 changes: 13 additions & 3 deletions components/drive/chromeos/change_list_loader.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,25 @@ class ChangeListLoader {
// ================= Implementation for change list loading =================

// Part of LoadFromServerIfNeeded().
// Starts loading the change list since |start_changestamp|, or the full
// resource list if |start_changestamp| is zero.
void LoadChangeListFromServer(int64_t start_changestamp);
// Starts loading the change list since |local_changestamp|, or the full
// resource list if |start_changestamp| is zero. If there's no changes since
// then, and there are no new team drives changes to apply from
// team_drives_change_lists, finishes early.
// TODO(sashab): Currently, team_drives_change_lists always contains all of
// the team drives. Update this so team_drives_change_lists is only filled
// when the TD flag is newly turned on or local data cleared. crbug.com/829154
void LoadChangeListFromServer(
std::unique_ptr<google_apis::AboutResource> about_resource,
int64_t local_changestamp,
FileError error,
std::vector<std::unique_ptr<ChangeList>> team_drives_change_lists);

// Part of LoadChangeListFromServer().
// Called when the entire change list is loaded.
void LoadChangeListFromServerAfterLoadChangeList(
std::unique_ptr<google_apis::AboutResource> about_resource,
bool is_delta_update,
std::vector<std::unique_ptr<ChangeList>> team_drives_change_lists,
FileError error,
std::vector<std::unique_ptr<ChangeList>> change_lists);

Expand Down

0 comments on commit 02a37b8

Please sign in to comment.