Skip to content

Commit

Permalink
[M116] printing: Fix primary user profile access
Browse files Browse the repository at this point in the history
CupsProxyServiceManager is KeyedService and is created during
profile loading. It creates CupsProxyServiceDelegateImpl when
CupsProxyClient thinks underlying dbus service is available.
CupsProxyServiceDelegateImpl reaches for primary user profile
in it constructor. There is a race between profile finish
loading and dbus service availability. When dbus availability
is signaled earlier than profile loading, DumpWithoutCrashing in
ProfileManager::GetPrimaryUserProfile would be hit.

Since CupsProxyServiceManager is a KeyedService and knows the
profile, we don't have to reach out to ProfileManager to get it.
This CL does the following:
- Create CupsProxyServiceManager only for the primary user profile;
- Pass down Profile explicitly to CupsProxyServiceDelegateImpl;

(cherry picked from commit 7e5575b)

Bug: 1424583
Change-Id: I6033318eff3d76126fceaeb901b57f8794c723e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4634497
Reviewed-by: Paul Moy <pmoy@chromium.org>
Reviewed-by: Benjamin Gordon <bmgordon@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1161308}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4646674
Auto-Submit: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Benjamin Gordon <bmgordon@chromium.org>
Cr-Commit-Position: refs/branch-heads/5845@{#100}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
Xiyuan Xia authored and Chromium LUCI CQ committed Jun 26, 2023
1 parent f642bc8 commit 01735cb
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
#include "chrome/browser/ash/printing/cups_printers_manager_factory.h"
#include "chrome/browser/ash/printing/printer_configurer.h"
#include "chrome/browser/printing/print_preview_sticky_settings.h"
#include "chrome/browser/profiles/profile_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/pref_names.h"
#include "components/prefs/pref_service.h"
#include "content/public/browser/browser_task_traits.h"
Expand All @@ -24,9 +24,8 @@ namespace ash {

using ::chromeos::Printer;

// TODO(crbug.com/945409): Decide on correct profile/s to use.
CupsProxyServiceDelegateImpl::CupsProxyServiceDelegateImpl()
: profile_(ProfileManager::GetPrimaryUserProfile()),
CupsProxyServiceDelegateImpl::CupsProxyServiceDelegateImpl(Profile* profile)
: profile_(profile),
printers_manager_(
CupsPrintersManagerFactory::GetForBrowserContext(profile_)) {
DETACH_FROM_SEQUENCE(sequence_checker_);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class PrinterConfigurer;
class CupsProxyServiceDelegateImpl
: public cups_proxy::CupsProxyServiceDelegate {
public:
CupsProxyServiceDelegateImpl();
explicit CupsProxyServiceDelegateImpl(Profile* profile);
~CupsProxyServiceDelegateImpl() override;

bool IsPrinterAccessAllowed() const override;
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/ash/printing/cups_proxy_service_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

namespace ash {

CupsProxyServiceManager::CupsProxyServiceManager() {
CupsProxyServiceManager::CupsProxyServiceManager(Profile* profile)
: profile_(profile) {
// Don't wait for the daemon if the feature is turned off anyway.
if (base::FeatureList::IsEnabled(features::kPluginVm)) {
CupsProxyClient::Get()->WaitForServiceToBeAvailable(
Expand All @@ -34,7 +35,7 @@ void CupsProxyServiceManager::OnDaemonAvailable(bool daemon_available) {
// Attempt to start the service, which will then bootstrap a connection
// with the daemon.
cups_proxy::CupsProxyService::Spawn(
std::make_unique<CupsProxyServiceDelegateImpl>());
std::make_unique<CupsProxyServiceDelegateImpl>(profile_));
}

} // namespace ash
6 changes: 5 additions & 1 deletion chrome/browser/ash/printing/cups_proxy_service_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
#ifndef CHROME_BROWSER_ASH_PRINTING_CUPS_PROXY_SERVICE_MANAGER_H_
#define CHROME_BROWSER_ASH_PRINTING_CUPS_PROXY_SERVICE_MANAGER_H_

#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chrome/services/cups_proxy/cups_proxy_service.h"
#include "components/keyed_service/core/keyed_service.h"

class Profile;

namespace ash {

// This KeyedService is responsible for helping manage the
Expand All @@ -21,7 +24,7 @@ namespace ash {
// fail, we do not try to restart.
class CupsProxyServiceManager : public KeyedService {
public:
CupsProxyServiceManager();
explicit CupsProxyServiceManager(Profile* profile);

CupsProxyServiceManager(const CupsProxyServiceManager&) = delete;
CupsProxyServiceManager& operator=(const CupsProxyServiceManager&) = delete;
Expand All @@ -31,6 +34,7 @@ class CupsProxyServiceManager : public KeyedService {
private:
void OnDaemonAvailable(bool daemon_available);

const raw_ptr<Profile, ExperimentalAsh> profile_;
base::WeakPtrFactory<CupsProxyServiceManager> weak_factory_{this};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

#include "base/no_destructor.h"
#include "chrome/browser/ash/printing/cups_proxy_service_manager.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/ash/components/browser_context_helper/browser_context_helper.h"
#include "components/user_manager/user_manager.h"

namespace ash {

Expand Down Expand Up @@ -36,7 +39,15 @@ CupsProxyServiceManagerFactory::~CupsProxyServiceManagerFactory() = default;

KeyedService* CupsProxyServiceManagerFactory::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new CupsProxyServiceManager();
// Only create the service for the primary user.
Profile* profile = Profile::FromBrowserContext(context);
auto* user =
ash::BrowserContextHelper::Get()->GetUserByBrowserContext(profile);
if (!user_manager::UserManager::Get()->IsPrimaryUser(user)) {
return nullptr;
}

return new CupsProxyServiceManager(profile);
}

bool CupsProxyServiceManagerFactory::ServiceIsCreatedWithBrowserContext()
Expand Down

0 comments on commit 01735cb

Please sign in to comment.