Commit c8d5ad5a authored by Eric Willigers's avatar Eric Willigers Committed by Commit Bot

[WIP] Rename PendingAppManagerImpl private members

Rename current_task_and_callback_ and pending_tasks_and_callbacks_
to current_install_ and pending_installs_.

Rename MaybeStartNextInstallation to MaybeStartNext.

This is in preparation for adding current_registration_ and
pending_registrations_. PendingAppManagerImpl will have two
queues, one for installs and one for service worker registrations.

Bug: 1757716
Change-Id: I0095ef151dec41be1b67fba32c3eea631d39297e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763562Reviewed-by: default avatarEric Willigers <ericwilligers@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689363}
parent 430d73d0
...@@ -40,27 +40,21 @@ PendingAppManagerImpl::~PendingAppManagerImpl() = default; ...@@ -40,27 +40,21 @@ PendingAppManagerImpl::~PendingAppManagerImpl() = default;
void PendingAppManagerImpl::Install(ExternalInstallOptions install_options, void PendingAppManagerImpl::Install(ExternalInstallOptions install_options,
OnceInstallCallback callback) { OnceInstallCallback callback) {
pending_tasks_and_callbacks_.push_front(std::make_unique<TaskAndCallback>( pending_installs_.push_front(std::make_unique<TaskAndCallback>(
CreateInstallationTask(std::move(install_options)), std::move(callback))); CreateInstallationTask(std::move(install_options)), std::move(callback)));
base::ThreadTaskRunnerHandle::Get()->PostTask( PostMaybeStartNext();
FROM_HERE,
base::BindOnce(&PendingAppManagerImpl::MaybeStartNextInstallation,
weak_ptr_factory_.GetWeakPtr()));
} }
void PendingAppManagerImpl::InstallApps( void PendingAppManagerImpl::InstallApps(
std::vector<ExternalInstallOptions> install_options_list, std::vector<ExternalInstallOptions> install_options_list,
const RepeatingInstallCallback& callback) { const RepeatingInstallCallback& callback) {
for (auto& install_options : install_options_list) { for (auto& install_options : install_options_list) {
pending_tasks_and_callbacks_.push_back(std::make_unique<TaskAndCallback>( pending_installs_.push_back(std::make_unique<TaskAndCallback>(
CreateInstallationTask(std::move(install_options)), callback)); CreateInstallationTask(std::move(install_options)), callback));
} }
base::ThreadTaskRunnerHandle::Get()->PostTask( PostMaybeStartNext();
FROM_HERE,
base::BindOnce(&PendingAppManagerImpl::MaybeStartNextInstallation,
weak_ptr_factory_.GetWeakPtr()));
} }
void PendingAppManagerImpl::UninstallApps(std::vector<GURL> uninstall_urls, void PendingAppManagerImpl::UninstallApps(std::vector<GURL> uninstall_urls,
...@@ -75,8 +69,8 @@ void PendingAppManagerImpl::UninstallApps(std::vector<GURL> uninstall_urls, ...@@ -75,8 +69,8 @@ void PendingAppManagerImpl::UninstallApps(std::vector<GURL> uninstall_urls,
} }
void PendingAppManagerImpl::Shutdown() { void PendingAppManagerImpl::Shutdown() {
pending_tasks_and_callbacks_.clear(); pending_installs_.clear();
current_task_and_callback_.reset(); current_install_.reset();
web_contents_.reset(); web_contents_.reset();
} }
...@@ -93,14 +87,20 @@ PendingAppManagerImpl::CreateInstallationTask( ...@@ -93,14 +87,20 @@ PendingAppManagerImpl::CreateInstallationTask(
std::move(install_options)); std::move(install_options));
} }
void PendingAppManagerImpl::MaybeStartNextInstallation() { void PendingAppManagerImpl::PostMaybeStartNext() {
if (current_task_and_callback_) base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, base::BindOnce(&PendingAppManagerImpl::MaybeStartNext,
weak_ptr_factory_.GetWeakPtr()));
}
void PendingAppManagerImpl::MaybeStartNext() {
if (current_install_)
return; return;
while (!pending_tasks_and_callbacks_.empty()) { while (!pending_installs_.empty()) {
std::unique_ptr<TaskAndCallback> front = std::unique_ptr<TaskAndCallback> front =
std::move(pending_tasks_and_callbacks_.front()); std::move(pending_installs_.front());
pending_tasks_and_callbacks_.pop_front(); pending_installs_.pop_front();
const ExternalInstallOptions& install_options = const ExternalInstallOptions& install_options =
front->task->install_options(); front->task->install_options();
...@@ -169,12 +169,12 @@ void PendingAppManagerImpl::MaybeStartNextInstallation() { ...@@ -169,12 +169,12 @@ void PendingAppManagerImpl::MaybeStartNextInstallation() {
void PendingAppManagerImpl::StartInstallationTask( void PendingAppManagerImpl::StartInstallationTask(
std::unique_ptr<TaskAndCallback> task) { std::unique_ptr<TaskAndCallback> task) {
DCHECK(!current_task_and_callback_); DCHECK(!current_install_);
current_task_and_callback_ = std::move(task); current_install_ = std::move(task);
CreateWebContentsIfNecessary(); CreateWebContentsIfNecessary();
url_loader_->LoadUrl(current_task_and_callback_->task->install_options().url, url_loader_->LoadUrl(current_install_->task->install_options().url,
web_contents_.get(), web_contents_.get(),
base::BindOnce(&PendingAppManagerImpl::OnUrlLoaded, base::BindOnce(&PendingAppManagerImpl::OnUrlLoaded,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
...@@ -190,7 +190,7 @@ void PendingAppManagerImpl::CreateWebContentsIfNecessary() { ...@@ -190,7 +190,7 @@ void PendingAppManagerImpl::CreateWebContentsIfNecessary() {
} }
void PendingAppManagerImpl::OnUrlLoaded(WebAppUrlLoader::Result result) { void PendingAppManagerImpl::OnUrlLoaded(WebAppUrlLoader::Result result) {
current_task_and_callback_->task->Install( current_install_->task->Install(
web_contents_.get(), result, web_contents_.get(), result,
base::BindOnce(&PendingAppManagerImpl::OnInstalled, base::BindOnce(&PendingAppManagerImpl::OnInstalled,
weak_ptr_factory_.GetWeakPtr())); weak_ptr_factory_.GetWeakPtr()));
...@@ -206,15 +206,10 @@ void PendingAppManagerImpl::CurrentInstallationFinished( ...@@ -206,15 +206,10 @@ void PendingAppManagerImpl::CurrentInstallationFinished(
// Post a task to avoid InstallableManager crashing and do so before // Post a task to avoid InstallableManager crashing and do so before
// running the callback in case the callback tries to install another // running the callback in case the callback tries to install another
// app. // app.
// TODO(crbug.com/943848): Run next installation synchronously once PostMaybeStartNext();
// InstallableManager is fixed.
base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(&PendingAppManagerImpl::MaybeStartNextInstallation,
weak_ptr_factory_.GetWeakPtr()));
std::unique_ptr<TaskAndCallback> task_and_callback; std::unique_ptr<TaskAndCallback> task_and_callback;
task_and_callback.swap(current_task_and_callback_); task_and_callback.swap(current_install_);
std::move(task_and_callback->callback) std::move(task_and_callback->callback)
.Run(task_and_callback->task->install_options().url, code); .Run(task_and_callback->task->install_options().url, code);
} }
......
...@@ -5,12 +5,12 @@ ...@@ -5,12 +5,12 @@
#ifndef CHROME_BROWSER_WEB_APPLICATIONS_PENDING_APP_MANAGER_IMPL_H_ #ifndef CHROME_BROWSER_WEB_APPLICATIONS_PENDING_APP_MANAGER_IMPL_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_PENDING_APP_MANAGER_IMPL_H_ #define CHROME_BROWSER_WEB_APPLICATIONS_PENDING_APP_MANAGER_IMPL_H_
#include <deque>
#include <memory> #include <memory>
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/callback.h" #include "base/callback.h"
#include "base/containers/circular_deque.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
...@@ -59,7 +59,9 @@ class PendingAppManagerImpl : public PendingAppManager { ...@@ -59,7 +59,9 @@ class PendingAppManagerImpl : public PendingAppManager {
private: private:
struct TaskAndCallback; struct TaskAndCallback;
void MaybeStartNextInstallation(); void PostMaybeStartNext();
void MaybeStartNext();
void StartInstallationTask(std::unique_ptr<TaskAndCallback> task); void StartInstallationTask(std::unique_ptr<TaskAndCallback> task);
...@@ -76,7 +78,7 @@ class PendingAppManagerImpl : public PendingAppManager { ...@@ -76,7 +78,7 @@ class PendingAppManagerImpl : public PendingAppManager {
void CurrentInstallationFinished(const base::Optional<std::string>& app_id, void CurrentInstallationFinished(const base::Optional<std::string>& app_id,
InstallResultCode code); InstallResultCode code);
Profile* profile_; Profile* const profile_;
ExternallyInstalledWebAppPrefs externally_installed_app_prefs_; ExternallyInstalledWebAppPrefs externally_installed_app_prefs_;
// unique_ptr so that it can be replaced in tests. // unique_ptr so that it can be replaced in tests.
...@@ -84,9 +86,9 @@ class PendingAppManagerImpl : public PendingAppManager { ...@@ -84,9 +86,9 @@ class PendingAppManagerImpl : public PendingAppManager {
std::unique_ptr<content::WebContents> web_contents_; std::unique_ptr<content::WebContents> web_contents_;
std::unique_ptr<TaskAndCallback> current_task_and_callback_; std::unique_ptr<TaskAndCallback> current_install_;
std::deque<std::unique_ptr<TaskAndCallback>> pending_tasks_and_callbacks_; base::circular_deque<std::unique_ptr<TaskAndCallback>> pending_installs_;
base::WeakPtrFactory<PendingAppManagerImpl> weak_ptr_factory_{this}; base::WeakPtrFactory<PendingAppManagerImpl> weak_ptr_factory_{this};
......
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