Enterprise Printers: Bugfix for missing printers (Device and User policies)

Since M-78, external policies are initialized after CupsPrintersManager. It
causes a problem with Enterprise Printers, because BulkPrinterCalculators are
created during external policies initialization. Since M-78,
EnterprisePrintersProvider (started by CupsPrintersManager) cannot find
objects of BulkPrinterCalculators and always returns empty list of printers.
This patch solves this problem by changing the lifetime of
BulkPrintersCalculator. From now, each BulkPrintersCalculator is created by
the first object that asks for it.
Some unittests require that BulkPrintersCalculator are deleted at the end of
each unittest. The patch solving that problem is in CL:1836660.

BUG=chromium:1010289,chromium:1009062
TEST=on nautilus

Change-Id: Ie9b06f05a67f028204230b6b84162a058457fbd4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1838051Reviewed-by: Sergey Poromov <poromov@chromium.org>
Reviewed-by: Sean Kau <skau@chromium.org>
Commit-Queue: Piotr Pawliczek <pawliczek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703123}

chrome/browser/chromeos/policy/external_data_handlers/device_native_printers_external_data_handler.cc

@@ -16,8 +16,7 @@ namespace policy {
 namespace {
 
 base::WeakPtr<chromeos::BulkPrintersCalculator> GetBulkPrintersCalculator() {
-  return chromeos::BulkPrintersCalculatorFactory::Get()->GetForDevice(
-      /*create_if_not_exists=*/true);
+  return chromeos::BulkPrintersCalculatorFactory::Get()->GetForDevice();
 }
 
 }  // namespace

chrome/browser/chromeos/policy/external_data_handlers/device_native_printers_external_data_handler_unittest.cc

@@ -74,7 +74,7 @@ class DeviceNativePrintersExternalDataHandlerTest : public testing::Test {
         std::make_unique<DeviceNativePrintersExternalDataHandler>(
             &policy_service_);
     external_printers_ =
-        chromeos::BulkPrintersCalculatorFactory::Get()->GetForDevice(true);
+        chromeos::BulkPrintersCalculatorFactory::Get()->GetForDevice();
     external_printers_->SetAccessMode(
         chromeos::BulkPrintersCalculator::ALL_ACCESS);
   }

chrome/browser/chromeos/policy/external_data_handlers/native_printers_external_data_handler.cc

@@ -19,8 +19,7 @@ namespace {
 base::WeakPtr<chromeos::BulkPrintersCalculator> GetBulkPrintersCalculator(
     const std::string& user_id) {
   return chromeos::BulkPrintersCalculatorFactory::Get()->GetForAccountId(
-      CloudExternalDataPolicyHandler::GetAccountId(user_id),
-      /*create_if_not_exists=*/true);
+      CloudExternalDataPolicyHandler::GetAccountId(user_id));
 }
 
 }  // namespace

chrome/browser/chromeos/printing/bulk_printers_calculator_factory.cc

@@ -20,27 +20,23 @@ BulkPrintersCalculatorFactory* BulkPrintersCalculatorFactory::Get() {
 }
 
 base::WeakPtr<BulkPrintersCalculator>
-BulkPrintersCalculatorFactory::GetForAccountId(const AccountId& account_id,
-                                               bool create_if_not_exists) {
+BulkPrintersCalculatorFactory::GetForAccountId(const AccountId& account_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   auto it = printers_by_user_.find(account_id);
   if (it != printers_by_user_.end())
     return it->second->AsWeakPtr();
-  if (!create_if_not_exists)
-    return nullptr;
   printers_by_user_.emplace(account_id, BulkPrintersCalculator::Create());
   return printers_by_user_[account_id]->AsWeakPtr();
 }
 
 base::WeakPtr<BulkPrintersCalculator>
-BulkPrintersCalculatorFactory::GetForProfile(Profile* profile,
-                                             bool create_if_not_exists) {
+BulkPrintersCalculatorFactory::GetForProfile(Profile* profile) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   const user_manager::User* user =
       ProfileHelper::Get()->GetUserByProfile(profile);
   if (!user)
     return nullptr;
-  return GetForAccountId(user->GetAccountId(), create_if_not_exists);
+  return GetForAccountId(user->GetAccountId());
 }
 
 void BulkPrintersCalculatorFactory::RemoveForUserId(
@@ -50,12 +46,10 @@ void BulkPrintersCalculatorFactory::RemoveForUserId(
 }
 
 base::WeakPtr<BulkPrintersCalculator>
-BulkPrintersCalculatorFactory::GetForDevice(bool create_if_not_exists) {
+BulkPrintersCalculatorFactory::GetForDevice() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (device_printers_)
     return device_printers_->AsWeakPtr();
-  if (!create_if_not_exists)
-    return nullptr;
   device_printers_ = BulkPrintersCalculator::Create();
   return device_printers_->AsWeakPtr();
 }

chrome/browser/chromeos/printing/bulk_printers_calculator_factory.h

@@ -30,32 +30,24 @@ class BulkPrintersCalculatorFactory {
 
   // Returns a WeakPtr to the BulkPrintersCalculator registered for
   // |account_id|.
-  // If requested BulkPrintersCalculator does not exist, the output depends on
-  // the given parameter |create_if_not_exists|. If it is true, the object is
-  // created and registered, otherwise nullptr is returned.
-  // The returned object remains valid until RemoveForUserId or Shutdown is
-  // called.
+  // If requested BulkPrintersCalculator does not exist, the object is
+  // created and registered. The returned object remains valid until
+  // RemoveForUserId or Shutdown is called.
   base::WeakPtr<BulkPrintersCalculator> GetForAccountId(
-      const AccountId& account_id,
-      bool create_if_not_exists);
+      const AccountId& account_id);
 
   // Returns a WeakPtr to the BulkPrintersCalculator registered for |profile|
   // which could be nullptr if |profile| does not map to a valid AccountId.
-  // If requested BulkPrintersCalculator does not exist, the output depends on
-  // the given parameter |create_if_not_exists|. If it is true, the object is
-  // created and registered, otherwise nullptr is returned.
-  // The returned object remains valid until RemoveForUserId or Shutdown is
-  // called.
-  base::WeakPtr<BulkPrintersCalculator> GetForProfile(
-      Profile* profile,
-      bool create_if_not_exists);
+  // If requested BulkPrintersCalculator does not exist, the object is
+  // created and registered. The returned object remains valid until
+  // RemoveForUserId or Shutdown is called.
+  base::WeakPtr<BulkPrintersCalculator> GetForProfile(Profile* profile);
 
   // Returns a WeakPtr to the BulkPrintersCalculator registered for the device.
-  // If requested BulkPrintersCalculator does not exist, the output depends on
-  // the given parameter |create_if_not_exists|. If it is true, the object is
-  // created and registered, otherwise nullptr is returned.
-  // The returned object remains valid until Shutdown is called.
-  base::WeakPtr<BulkPrintersCalculator> GetForDevice(bool create_if_not_exists);
+  // If requested BulkPrintersCalculator does not exist, the object is
+  // created and registered. The returned object remains valid until Shutdown is
+  // called.
+  base::WeakPtr<BulkPrintersCalculator> GetForDevice();
 
   // Deletes the BulkPrintersCalculator registered for |account_id|.
   void RemoveForUserId(const AccountId& account_id);

chrome/browser/chromeos/printing/calculators_policies_binder.cc

@@ -60,8 +60,7 @@ class CalculatorsPoliciesBinderImpl : public CalculatorsPoliciesBinder {
       : settings_(settings), profile_(profile) {
     pref_change_registrar_.Init(profile->GetPrefs());
     // Bind device policies to corresponding instance of BulkPrintersCalculator.
-    device_printers_ = BulkPrintersCalculatorFactory::Get()->GetForDevice(
-        /*create_if_not_exists=*/false);
+    device_printers_ = BulkPrintersCalculatorFactory::Get()->GetForDevice();
     if (device_printers_ && ++(BindingsCount()[device_printers_.get()]) == 1) {
       BindSettings(kDeviceNativePrintersAccessMode,
                    &CalculatorsPoliciesBinderImpl::UpdateDeviceAccessMode);
@@ -71,9 +70,8 @@ class CalculatorsPoliciesBinderImpl : public CalculatorsPoliciesBinder {
                    &CalculatorsPoliciesBinderImpl::UpdateDeviceWhitelist);
     }
     // Bind user policies to corresponding instance of BulkPrintersCalculator.
-    user_printers_ = BulkPrintersCalculatorFactory::Get()->GetForProfile(
-        profile,
-        /*create_if_not_exists=*/false);
+    user_printers_ =
+        BulkPrintersCalculatorFactory::Get()->GetForProfile(profile);
     if (user_printers_ && ++(BindingsCount()[user_printers_.get()]) == 1) {
       BindPref(prefs::kRecommendedNativePrintersAccessMode,
                &CalculatorsPoliciesBinderImpl::UpdateUserAccessMode);

chrome/browser/chromeos/printing/enterprise_printers_provider.cc

@@ -49,16 +49,14 @@ class EnterprisePrintersProviderImpl : public EnterprisePrintersProvider,
     // Binds instances of BulkPrintersCalculator to policies.
     policies_binder_ = CalculatorsPoliciesBinder::Create(settings, profile);
     // Get instance of BulkPrintersCalculator for device policies.
-    device_printers_ = BulkPrintersCalculatorFactory::Get()->GetForDevice(
-        /*create_if_not_exists=*/false);
+    device_printers_ = BulkPrintersCalculatorFactory::Get()->GetForDevice();
     if (device_printers_) {
       device_printers_->AddObserver(this);
       RecalculateCompleteFlagForDevicePrinters();
     }
     // Get instance of BulkPrintersCalculator for user policies.
-    user_printers_ = BulkPrintersCalculatorFactory::Get()->GetForProfile(
-        profile,
-        /*create_if_not_exists=*/false);
+    user_printers_ =
+        BulkPrintersCalculatorFactory::Get()->GetForProfile(profile);
     if (user_printers_) {
       user_printers_->AddObserver(this);
       RecalculateCompleteFlagForUserPrinters();