Skip to content

Commit

Permalink
M98 merge: [Extensions] Fix a null dereference in CrxInstaller
Browse files Browse the repository at this point in the history
This class holds a ScopedProfileKeepAlive, which points to a Profile.
This SPKA can be destroy too late, if Profile is destroyed before it.
This can only happen if Chrome is shutting down entirely, but we need
to perform some cleanup in that case.

(cherry picked from commit fa1f6c3)

Bug: 1291767
Change-Id: Id6c411edcf4fe8d7d0bd0d941de9180e95b08e4e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3424799
Commit-Queue: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Auto-Submit: Nicolas Ouellet-Payeur <nicolaso@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#964665}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3450858
Cr-Commit-Position: refs/branch-heads/4758@{#1124}
Cr-Branched-From: 4a2cf4b-refs/heads/main@{#950365}
  • Loading branch information
Nicolas Ouellet-Payeur authored and Chromium LUCI CQ committed Feb 9, 2022
1 parent 2b608b9 commit 1abe4bb
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
8 changes: 8 additions & 0 deletions chrome/browser/extensions/crx_installer.cc
Expand Up @@ -129,6 +129,8 @@ CrxInstaller::CrxInstaller(base::WeakPtr<ExtensionService> service_weak,
shared_file_task_runner_(GetExtensionFileTaskRunner()),
update_from_settings_page_(false),
install_flags_(kInstallFlagNone) {
profile_observation_.Observe(profile_);

if (!approval)
return;

Expand Down Expand Up @@ -601,6 +603,12 @@ void CrxInstaller::OnStageChanged(InstallationStage stage) {
ReportInstallationStage(stage);
}

void CrxInstaller::OnProfileWillBeDestroyed(Profile* profile) {
DCHECK_EQ(profile, profile_);
profile_keep_alive_.reset();
profile_observation_.Reset();
}

void CrxInstaller::CheckInstall() {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
ExtensionService* service = service_weak_.get();
Expand Down
10 changes: 9 additions & 1 deletion chrome/browser/extensions/crx_installer.h
Expand Up @@ -14,10 +14,12 @@
#include "base/memory/raw_ptr.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
#include "base/scoped_observation.h"
#include "base/version.h"
#include "chrome/browser/extensions/extension_install_prompt.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/webstore_installer.h"
#include "chrome/browser/profiles/profile_observer.h"
#include "chrome/common/extensions/extension_constants.h"
#include "components/sync/model/string_ordinal.h"
#include "extensions/browser/api/declarative_net_request/ruleset_install_pref.h"
Expand Down Expand Up @@ -72,7 +74,7 @@ class PreloadCheckGroup;
// terminating during the install. We can't listen for the app termination
// notification here in this class because it can be destroyed on any thread
// and won't safely be able to clean up UI thread notification listeners.
class CrxInstaller : public SandboxedUnpackerClient {
class CrxInstaller : public SandboxedUnpackerClient, public ProfileObserver {
public:
// A callback to be executed when the install finishes.
using InstallerResultCallback = ExtensionSystem::InstallUpdateCallback;
Expand Down Expand Up @@ -302,6 +304,9 @@ class CrxInstaller : public SandboxedUnpackerClient {
ruleset_install_prefs) override;
void OnStageChanged(InstallationStage stage) override;

// ProfileObserver
void OnProfileWillBeDestroyed(Profile* profile) override;

// Called on the UI thread to start the requirements, policy and blocklist
// checks on the extension.
void CheckInstall();
Expand Down Expand Up @@ -371,6 +376,9 @@ class CrxInstaller : public SandboxedUnpackerClient {

// Prevent Profile destruction until the CrxInstaller is done.
std::unique_ptr<ScopedProfileKeepAlive> profile_keep_alive_;
// ... but |profile_| could still get destroyed early, if Chrome shuts down
// completely. We need to perform some cleanup if that happens.
base::ScopedObservation<Profile, ProfileObserver> profile_observation_{this};

// The extension being installed.
scoped_refptr<const Extension> extension_;
Expand Down

0 comments on commit 1abe4bb

Please sign in to comment.