Skip to content

Commit

Permalink
[Extensions] Refactor
Browse files Browse the repository at this point in the history
InstalledLoader::ProfileCanUseNonComponentExtensions into its own file.

Before this the method was coupled to InstalledLoader so any potential
importing code would have to import it all. We are going to start using
this method outside of InstalledLoader in a future CL so this does the
decoupling for that.

Bug: 1383740
Change-Id: I7089276ff8dbc37b5348d9931692637b7d7cc6fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4125478
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Commit-Queue: Justin Lulejian <jlulejian@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1100748}
  • Loading branch information
Justin Lulejian authored and Chromium LUCI CQ committed Feb 3, 2023
1 parent d9da963 commit d198df2
Show file tree
Hide file tree
Showing 10 changed files with 353 additions and 430 deletions.
2 changes: 2 additions & 0 deletions chrome/browser/extensions/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,8 @@ static_library("extensions") {
"pref_mapping.cc",
"pref_mapping.h",
"pref_transformer_interface.h",
"profile_util.cc",
"profile_util.h",
"proxy_overridden_bubble_delegate.cc",
"proxy_overridden_bubble_delegate.h",
"safe_browsing_verdict_handler.cc",
Expand Down
58 changes: 58 additions & 0 deletions chrome/browser/extensions/extension_service_user_test_base.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/extensions/extension_service_user_test_base.h"
#include "chrome/test/base/testing_profile.h"

namespace extensions {

ExtensionServiceUserTestBase::ExtensionServiceUserTestBase() = default;
ExtensionServiceUserTestBase::~ExtensionServiceUserTestBase() = default;

#if BUILDFLAG(IS_CHROMEOS_ASH)
void ExtensionServiceUserTestBase::SetUp() {
ExtensionServiceTestBase::SetUp();
scoped_user_manager_ = std::make_unique<user_manager::ScopedUserManager>(
std::make_unique<ash::FakeChromeUserManager>());
account_id_ =
AccountId::FromUserEmailGaiaId("test-user@testdomain.com", "1234567890");
}

void ExtensionServiceUserTestBase::TearDown() {
ExtensionServiceTestBase::TearDown();
profile_.reset();
scoped_user_manager_.reset();
}

void ExtensionServiceUserTestBase::LoginChromeOSAshUser(
const user_manager::User* user,
const AccountId& account_id) {
ASSERT_TRUE(user);
GetFakeUserManager()->LoginUser(account_id,
/*set_profile_created_flag=*/false);
ASSERT_TRUE(GetFakeUserManager()->IsUserLoggedIn());
ASSERT_TRUE(user == GetFakeUserManager()->GetActiveUser());
}

#endif // BUILDFLAG(IS_CHROMEOS_ASH)

void ExtensionServiceUserTestBase::MaybeSetUpTestUser(bool is_guest) {
profile_->SetGuestSession(is_guest);

ASSERT_EQ(is_guest, profile_->IsGuestSession());

#if BUILDFLAG(IS_CHROMEOS_ASH)
user_manager::User* user;
AccountId account_id = account_id_;
if (is_guest) {
user = GetFakeUserManager()->AddGuestUser();
account_id = GetFakeUserManager()->GetGuestAccountId();
} else {
user = GetFakeUserManager()->AddUser(account_id_);
}
ASSERT_NO_FATAL_FAILURE(LoginChromeOSAshUser(user, account_id));
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
}

} // namespace extensions
55 changes: 55 additions & 0 deletions chrome/browser/extensions/extension_service_user_test_base.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2023 The Chromium Authors
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_USER_TEST_BASE_H_
#define CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_USER_TEST_BASE_H_

#include "chrome/browser/extensions/extension_service_test_base.h"

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "chrome/browser/ash/login/users/fake_chrome_user_manager.h"
#include "components/user_manager/scoped_user_manager.h"
#endif

namespace extensions {

// Test class used to setup test users in the unit test for browser/lacros and
// ChromeOS Ash.
class ExtensionServiceUserTestBase : public ExtensionServiceTestBase {
public:
ExtensionServiceUserTestBase();
~ExtensionServiceUserTestBase() override;

#if BUILDFLAG(IS_CHROMEOS_ASH)
void SetUp() override;

void TearDown() override;

void LoginChromeOSAshUser(const user_manager::User* user,
const AccountId& account_id);

ash::FakeChromeUserManager* GetFakeUserManager() const {
return static_cast<ash::FakeChromeUserManager*>(
user_manager::UserManager::Get());
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

// If browser/lacros: set the profile for the test as a guest if `is_guest` is
// `true`. If ChromeOS Ash: do the above, but also login a
// `user_manager::User` and set it to be a guest account if `is_guest` is
// `true`.
void MaybeSetUpTestUser(bool is_guest);

#if BUILDFLAG(IS_CHROMEOS_ASH)
protected:
AccountId account_id_;

private:
std::unique_ptr<user_manager::ScopedUserManager> scoped_user_manager_;
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
};

} // namespace extensions

#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_USER_TEST_BASE_H_
95 changes: 6 additions & 89 deletions chrome/browser/extensions/installed_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_util.h"
#include "chrome/browser/extensions/load_error_reporter.h"
#include "chrome/browser/extensions/profile_util.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/common/extensions/chrome_manifest_url_handlers.h"
Expand Down Expand Up @@ -362,48 +363,17 @@ void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) {
extension_service_->AddExtension(extension.get());
}

#if BUILDFLAG(IS_CHROMEOS_ASH)
void InstalledLoader::LoadAllExtensions() {
LoadAllExtensions(extension_service_->profile(), ash::ProfileHelper::Get());
}
#else
void InstalledLoader::LoadAllExtensions() {
LoadAllExtensions(extension_service_->profile());
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_CHROMEOS_ASH)
void InstalledLoader::LoadAllExtensions(Profile* profile,
ash::ProfileHelper* profile_helper) {
#else
void InstalledLoader::LoadAllExtensions(Profile* profile) {
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
DCHECK_CURRENTLY_ON(BrowserThread::UI);
TRACE_EVENT0("browser,startup", "InstalledLoader::LoadAllExtensions");

// TODO(crbug.com/1383740): Have profile checking logic handle profile_helper
// swapping by itself.
#if BUILDFLAG(IS_CHROMEOS_ASH)
// These two conditionals allow testing code to pass in a mock/fake.
if (!profile_helper) {
profile_helper = ash::ProfileHelper::Get();
}
if (!profile) {
profile = extension_service_->profile();
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
bool should_record_incremented_metrics =
profile_util::ProfileCanUseNonComponentExtensions(profile);

bool profile_can_use_non_component_extensions = false;

#if BUILDFLAG(IS_CHROMEOS_ASH)
profile_can_use_non_component_extensions =
ProfileCanUseNonComponentExtensions(profile, profile_helper);
#else
profile_can_use_non_component_extensions =
ProfileCanUseNonComponentExtensions(profile);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

// TODO(crbug.com/1383740): Split this into user and non-user profile metrics.
SCOPED_UMA_HISTOGRAM_TIMER("Extensions.LoadAllTime2");

std::unique_ptr<ExtensionPrefs::ExtensionsInfo> extensions_info(
Expand Down Expand Up @@ -458,78 +428,25 @@ void InstalledLoader::LoadAllExtensions(Profile* profile) {
extension_registry_->enabled_extensions().size());
UMA_HISTOGRAM_COUNTS_100("Extensions.Disabled",
extension_registry_->disabled_extensions().size());
if (profile_can_use_non_component_extensions) {
if (should_record_incremented_metrics) {
UMA_HISTOGRAM_COUNTS_100("Extensions.LoadAll2",
extension_registry_->enabled_extensions().size());
UMA_HISTOGRAM_COUNTS_100("Extensions.Disabled2",
extension_registry_->disabled_extensions().size());
}

RecordExtensionsMetrics(profile, profile_can_use_non_component_extensions);
RecordExtensionsMetrics(profile, should_record_incremented_metrics);
}

void InstalledLoader::RecordExtensionsMetricsForTesting() {
RecordExtensionsMetrics(/*profile=*/extension_service_->profile(),
/*log_user_profile_histograms=*/false);
}

#if BUILDFLAG(IS_CHROMEOS_ASH)
void InstalledLoader::RecordExtensionsProfileSpecificMetricsForTesting(
Profile* profile,
ash::ProfileHelper* profile_helper) {
LoadAllExtensions(profile, profile_helper);
}
#else
void InstalledLoader::RecordExtensionsProfileSpecificMetricsForTesting(
void InstalledLoader::RecordExtensionsIncrementedMetricsForTesting(
Profile* profile) {
LoadAllExtensions(profile);
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

#if BUILDFLAG(IS_CHROMEOS_ASH)
// static
bool InstalledLoader::ProfileCanUseNonComponentExtensions(
const Profile* profile,
ash::ProfileHelper* profile_helper) {
if (!profile || !profile_helper ||
!ash::ProfileHelper::IsUserProfile(profile)) {
return false;
}

const user_manager::User* user = profile_helper->GetUserByProfile(profile);
if (!user) {
return false;
}

// ChromeOS has special irregular profiles that must also be filtered
// out in addition to `ProfileHelper::IsUserProfile()`. `IsUserProfile()`
// includes guest and public users (which cannot use non-component
// extensions) so instead only look for those user types that can use them.
switch (user->GetType()) {
case user_manager::USER_TYPE_REGULAR:
case user_manager::USER_TYPE_CHILD:
case user_manager::USER_TYPE_ACTIVE_DIRECTORY:
return true;

case user_manager::USER_TYPE_GUEST:
case user_manager::USER_TYPE_PUBLIC_ACCOUNT:
case user_manager::USER_TYPE_KIOSK_APP:
case user_manager::USER_TYPE_ARC_KIOSK_APP:
case user_manager::USER_TYPE_WEB_KIOSK_APP:
case user_manager::NUM_USER_TYPES:
return false;
}
}
#else
// static
bool InstalledLoader::ProfileCanUseNonComponentExtensions(
const Profile* profile) {
if (!profile) {
return false;
}
return profile->IsRegularProfile();
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

// TODO(crbug.com/1163038): Separate out Webstore/Offstore metrics.
void InstalledLoader::RecordExtensionsMetrics(
Expand Down
39 changes: 3 additions & 36 deletions chrome/browser/extensions/installed_loader.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,8 @@

#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "build/chromeos_buildflags.h"

class Profile;
#if BUILDFLAG(IS_CHROMEOS_ASH)
namespace ash {
class ProfileHelper;
}
#endif // BUILDFLAG(IS_CHROMEOS_ASH)

namespace extensions {

Expand Down Expand Up @@ -53,42 +47,15 @@ class InstalledLoader {
// Loads all installed extensions (used by startup and testing code).
void LoadAllExtensions();

// Loads all installed extensions (used by startup and testing code).
// Loads all installed extensions (used by testing code).
void LoadAllExtensions(Profile* profile);

// Allows tests to verify metrics without needing to go through
// LoadAllExtensions().
void RecordExtensionsMetricsForTesting();

// Allows tests to verify incremented metrics without needing to go through
// LoadAllExtensions().
void RecordExtensionsProfileSpecificMetricsForTesting(Profile* profile);

#if BUILDFLAG(IS_CHROMEOS_ASH)
// Loads all installed extensions (used by testing code).
void LoadAllExtensions(Profile* profile, ash::ProfileHelper* profile_helper);

// Allows tests to verify incremented metrics without needing to go through
// LoadAllExtensions().
void RecordExtensionsProfileSpecificMetricsForTesting(
Profile* profile,
ash::ProfileHelper* profile_helper);

// TODO(crbug.com/1383740): Move ProfileCanUseNonComponentExtensions to
// another file in //chrome/browser/extensions.

// Returns true for ChromeOS Ash profiles that can use anything other than
// component extensions. It is required to provide a `profile_helper`
// otherwise this will always be `false`.
static bool ProfileCanUseNonComponentExtensions(
const Profile* profile,
ash::ProfileHelper* profile_helper);
#else
// Returns true for (browser and Lacros) profiles that can use anything other
// than component extensions. Lacros uses multi-profiles like the browser
// (go/multi-something).
static bool ProfileCanUseNonComponentExtensions(const Profile* profile);
#endif // BUILDFLAG(IS_CHROMEOS_ASH)
// Allows tests to verify incremented metrics.
void RecordExtensionsIncrementedMetricsForTesting(Profile* profile);

private:
// Returns the flags that should be used with Extension::Create() for an
Expand Down

0 comments on commit d198df2

Please sign in to comment.