Enable login screen apps.

chrome/browser/extensions/chrome_process_manager_delegate.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/chrome_process_manager_delegate.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/extension_management.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/common/chrome_switches.h"
 #include "components/user_manager/user_manager.h"
 #include "content/public/browser/notification_service.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/process_manager.h"
 #include "extensions/browser/process_manager_factory.h"
+#include "extensions/common/extension.h"
 #include "extensions/common/one_shot_event.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
+#endif
+
 namespace extensions {
 
 ChromeProcessManagerDelegate::ChromeProcessManagerDelegate() {
   registrar_.Add(this,
                  chrome::NOTIFICATION_BROWSER_WINDOW_READY,
                  content::NotificationService::AllSources());
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_CREATED,
                  content::NotificationService::AllSources());
   registrar_.Add(this,
                  chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::NotificationService::AllSources());
 }
 
 ChromeProcessManagerDelegate::~ChromeProcessManagerDelegate() {
 }
 
 bool ChromeProcessManagerDelegate::IsBackgroundPageAllowed(
-    content::BrowserContext* context) const {
+    content::BrowserContext* context,
+    const Extension* extension) const {
   Profile* profile = static_cast<Profile*>(context);
 
   bool is_normal_session = !profile->IsGuestSession() &&
                            !profile->IsSystemProfile();
+
 #if defined(OS_CHROMEOS)
-  is_normal_session = is_normal_session &&
-                      user_manager::UserManager::Get()->IsUserLoggedIn();
+  const bool is_signin_profile =
+      chromeos::ProfileHelper::IsSigninProfile(profile) &&
+      !profile->IsOffTheRecord();
+
+  if (is_signin_profile) {
+    // If an extension is specified, allow background pages only if that
+    // extension is in the login screen app list. Otherwise allow
+    // background pages.
+    bool is_login_screen_app = true;
+    if (extension) {

[emaxx, 2017/03/08 17:11:25]

Maybe reduce the nested level here by inverting this into
if (!extension)
  return false;
? Or you don't like introducing one more exit point here?

[Devlin, 2017/03/08 17:24:32]

When can |extension| be null?

[achuithb, 2017/03/09 14:31:05]

Done.

[achuithb, 2017/03/09 14:31:05]

I'll add a comment to the line where it's null

+      // Check for flag.
+      const bool login_screen_apps_enabled =
+          base::CommandLine::ForCurrentProcess()->HasSwitch(
+              switches::kEnableLoginScreenApps);
+
+      // Get login screen apps installed by policy.
+      std::unique_ptr<base::DictionaryValue> login_screen_apps_list =
+          ExtensionManagementFactory::GetForBrowserContext(context)
+              ->GetForceInstallList();
+
+      is_login_screen_app = login_screen_apps_enabled &&
+                            login_screen_apps_list->HasKey(extension->id());

[Devlin, 2017/03/08 17:24:32]

I would expect CreateBackgroundHost() to only be called for loaded extensions. 
How would the extension get loaded if it's not on the list of allowed
extensions?

[achuithb, 2017/03/09 14:31:05]

I don't entirely follow your question. 

We're actually blocking CreateBackgroundHost for all loaded extensions in the
signin profile except login screen apps specified by policy. This is the old
behavior.

[Devlin, 2017/03/13 19:39:35]

To rephrase my question: If this is the signin profile, which (my understanding
is) currently doesn't allow extensions, then why would we have loaded an
extension in that profile *unless* it is allowed by this policy?  Wouldn't that
make this check redundant?

[achuithb, 2017/03/15 00:40:25]

Component extensions are loaded into the signin profile. So this branch is
relevant for component extensions. Examples are gnubby extension, chromevox,
etc.

Background pages of component extensions are not allowed to execute here. They
are allowed to run in the OTR profile of the signin profile.

+    }
+
+    return is_login_screen_app;
+  }
+
+  is_normal_session =
+      is_normal_session && user_manager::UserManager::Get()->IsUserLoggedIn();
 #endif
 
   // Disallow if the current session is a Guest mode session or login screen but
   // the current browser context is *not* off-the-record. Such context is
   // artificial and background page shouldn't be created in it.
   // http://crbug.com/329498
   return is_normal_session || profile->IsOffTheRecord();
 }
 
 bool ChromeProcessManagerDelegate::DeferCreatingStartupBackgroundHosts(
@@ skipping 104 matching lines @@
     ProcessManager* incognito_manager =
         ProcessManagerFactory::GetForBrowserContextIfExists(
             profile->GetOffTheRecordProfile());
     if (incognito_manager) {
       incognito_manager->CloseBackgroundHosts();
     }
   }
 }
 
 }  // namespace extensions