Skip to content

Commit

Permalink
[M115] Fix segmentation fault on ExternallyManagedAppManager
Browse files Browse the repository at this point in the history
As written in https://crbug.com/1449075, ExternallyManagedAppManager has
a compatibility issue with kServiceWorkerRegistrationCache feature. This
CL fixes the "segmentation fault" on ExternallyManagedAppManager.

<How can this CL avoid segmentation fault>

This CL initializes current_registration_ field first, and call
CheckHasServiceWorker() afterwards so that we can ensure that
current_registration_ is not null.

(cherry picked from commit 5d67e82)

Bug: 1449075, 1446216
Change-Id: Id4026fa414d091f9d24725a474a93b59a0326142
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4567903
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1149554}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4570941
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/branch-heads/5790@{#125}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
chikamune-cr authored and Chromium LUCI CQ committed May 30, 2023
1 parent 2abab76 commit c231db4
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 5 deletions.
Expand Up @@ -247,7 +247,7 @@ ExternallyManagedAppManager::CreateInstallationTask(
}

std::unique_ptr<ExternallyManagedAppRegistrationTaskBase>
ExternallyManagedAppManager::StartRegistration(GURL install_url) {
ExternallyManagedAppManager::CreateRegistration(GURL install_url) {
DCHECK(!IsShuttingDown());
ExternallyManagedAppRegistrationTask::RegistrationCallback callback =
base::BindOnce(&ExternallyManagedAppManager::OnRegistrationFinished,
Expand Down Expand Up @@ -411,7 +411,8 @@ bool ExternallyManagedAppManager::RunNextRegistration() {

GURL url_to_check = std::move(pending_registrations_.front());
pending_registrations_.pop_front();
current_registration_ = StartRegistration(std::move(url_to_check));
current_registration_ = CreateRegistration(std::move(url_to_check));
current_registration_->Start();
return true;
}

Expand Down
Expand Up @@ -187,7 +187,7 @@ class ExternallyManagedAppManager {
CreateInstallationTask(ExternalInstallOptions install_options);

virtual std::unique_ptr<ExternallyManagedAppRegistrationTaskBase>
StartRegistration(GURL launch_url);
CreateRegistration(GURL launch_url);

virtual void OnRegistrationFinished(const GURL& launch_url,
RegistrationResultCode result);
Expand Down
Expand Up @@ -211,7 +211,7 @@ class TestExternallyManagedAppManager : public ExternallyManagedAppManager {
std::move(install_options));
}

std::unique_ptr<ExternallyManagedAppRegistrationTaskBase> StartRegistration(
std::unique_ptr<ExternallyManagedAppRegistrationTaskBase> CreateRegistration(
GURL install_url) override {
++registration_run_count_;
last_registered_install_url_ = install_url;
Expand Down Expand Up @@ -373,6 +373,8 @@ class TestExternallyManagedAppManager : public ExternallyManagedAppManager {
const TestExternallyManagedAppRegistrationTask&) = delete;
~TestExternallyManagedAppRegistrationTask() override = default;

void Start() override {}

private:
void OnProgress(const GURL& install_url) {
if (externally_managed_app_manager_impl_->MaybePreemptRegistration())
Expand Down
Expand Up @@ -35,7 +35,9 @@ ExternallyManagedAppRegistrationTask::ExternallyManagedAppRegistrationTask(
: ExternallyManagedAppRegistrationTaskBase(std::move(install_url)),
url_loader_(url_loader),
web_contents_(web_contents),
callback_(std::move(callback)) {
callback_(std::move(callback)) {}

void ExternallyManagedAppRegistrationTask::Start() {
content::StoragePartition* storage_partition =
web_contents_->GetBrowserContext()->GetStoragePartition(
web_contents_->GetSiteInstance());
Expand Down
Expand Up @@ -29,6 +29,7 @@ class WebAppUrlLoader;
class ExternallyManagedAppRegistrationTaskBase
: public content::ServiceWorkerContextObserver {
public:
virtual void Start() = 0;
~ExternallyManagedAppRegistrationTaskBase() override;

const GURL& install_url() const { return install_url_; }
Expand Down Expand Up @@ -59,6 +60,8 @@ class ExternallyManagedAppRegistrationTask
void OnRegistrationCompleted(const GURL& scope) override;
void OnDestruct(content::ServiceWorkerContext* context) override;

void Start() override;

static void SetTimeoutForTesting(int registration_timeout_in_seconds);

private:
Expand Down

0 comments on commit c231db4

Please sign in to comment.