Prevent loading of session extensions into the sign-in profile

Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified. This
happens in the following cases:
* On Chrome OS inside the user session after the browser has crashed at
  least once;
* In tests for Chrome OS that use stub or test user directly, without
  going through the login screen;
* In dev builds for Chrome OS running under Linux, when --login-manager
  is not passed via command line.

The effect of this bug is that:
1. Unwanted extensions were loaded, resulting in extra resources usage;
2. Log spam from signin_screen_policy_provider.cc with messages like
   "Denying load of Extension ...";
3. Error popups in tests that try to load extensions via command line
   when --enable-login-screen-apps was additionally specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464, 674316
TEST=extended browser tests for file manager extensions

Review-Url: https://codereview.chromium.org/2807683003
Cr-Commit-Position: refs/heads/master@{#463661}
Committed: https://chromium.googlesource.com/chromium/src/+/095cadb4ac6310a09f9f76b81fc0ae8784323a87

[emaxx, 2017-04-07 18:58:16]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-07 18:58:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[emaxx, 2017-04-07 19:12:58]

Description was changed from

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified (which
means that this is not affecting real world usages - only tests and
dev builds for Chrome OS under Linux). In these cases the following
happened:
* Unwanted extensions were loaded;
* Log spam from signin_screen_policy_provider.cc with messages like
  "Denying load of Extension ...";
* Error popups when --enable-login-screen-apps was specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended a browser test for file manager extensions
==========

to

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified (which
means that this is not affecting real world usages - only tests and
dev builds for Chrome OS under Linux). In these cases the following
happened:
* Unwanted extensions were loaded;
* Log spam from signin_screen_policy_provider.cc with messages like
  "Denying load of Extension ...";
* Error popups when --enable-login-screen-apps was specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions
==========

[commit-bot: I haz the power, 2017-04-07 20:16:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-07 20:16:55]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[emaxx, 2017-04-10 11:50:01]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 11:50:15]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 13:02:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 13:02:06]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[emaxx, 2017-04-10 13:05:24]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 13:05:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 13:09:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 13:09:40]

Dry run: Try jobs failed on following builders:
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[emaxx, 2017-04-10 14:13:12]

emaxx@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[emaxx, 2017-04-10 14:13:13]

Devlin, PTAL.
This is the second part of the fixes for the sign-in profile (the first was in
http://crrev.com/2800973005/).

[emaxx, 2017-04-10 14:13:19]

The CQ bit was checked by emaxx@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-10 14:13:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 14:20:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-10 14:21:00]

Dry run: This issue passed the CQ dry run.

[Devlin, 2017-04-10 15:40:54]

lgtm... this logic seems a little odd to me, but I know very little about
sessions and the login profile, so it's probably just me.

https://codereview.chromium.org/2807683003/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/extension_system_impl.cc (right):

https://codereview.chromium.org/2807683003/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/extension_system_impl.cc:250:
chromeos::ProfileHelper::IsSigninProfile(profile_);
It's surprising to me that the user can be logged in while the active profile is
the signin profile... is that intentional?

[emaxx, 2017-04-11 12:09:41]

Description was changed from

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified (which
means that this is not affecting real world usages - only tests and
dev builds for Chrome OS under Linux). In these cases the following
happened:
* Unwanted extensions were loaded;
* Log spam from signin_screen_policy_provider.cc with messages like
  "Denying load of Extension ...";
* Error popups when --enable-login-screen-apps was specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions
==========

to

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified. This
happens in the following cases:
* On Chrome OS inside the user session after the browser has crashed at
  least once;
* In tests for Chrome OS that use stub or test user directly, without
  passing through the login screen;
* In dev builds for Chrome OS running under Linux, when --login-manager
  is not passed via command line.

The effect of this bug is that:
1. Unwanted extensions were loaded, resulting in extra resources usage;
2. Log spam from signin_screen_policy_provider.cc with messages like
   "Denying load of Extension ...";
3. Error popups in tests that try to load extensions via command line
   when --enable-login-screen-apps was additionally specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions
==========

[emaxx, 2017-04-11 12:12:36]

On 2017/04/10 15:40:54, Devlin wrote:
> lgtm... this logic seems a little odd to me, but I know very little about
> sessions and the login profile, so it's probably just me.

Agreed that the stuff around sign-in profile is not properly documented,
but that's what we have now. I planned to work on some refactoring and
clean up around that piece in http://crbug.com/709474.

https://codereview.chromium.org/2807683003/diff/20001/chrome/browser/extensio...
File chrome/browser/extensions/extension_system_impl.cc (right):

https://codereview.chromium.org/2807683003/diff/20001/chrome/browser/extensio...
chrome/browser/extensions/extension_system_impl.cc:250:
chromeos::ProfileHelper::IsSigninProfile(profile_);
On 2017/04/10 15:40:54, Devlin wrote:
> It's surprising to me that the user can be logged in while the active profile
is
> the signin profile... is that intentional?

I think the sign-in profile (and the initial profile, which is the original
profile for the incognito sign-in profile) is always there. Maybe it's because
it hosts the stuff for the lock screen and the multiple sign-in.

[emaxx, 2017-04-11 12:14:46]

Description was changed from

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified. This
happens in the following cases:
* On Chrome OS inside the user session after the browser has crashed at
  least once;
* In tests for Chrome OS that use stub or test user directly, without
  passing through the login screen;
* In dev builds for Chrome OS running under Linux, when --login-manager
  is not passed via command line.

The effect of this bug is that:
1. Unwanted extensions were loaded, resulting in extra resources usage;
2. Log spam from signin_screen_policy_provider.cc with messages like
   "Denying load of Extension ...";
3. Error popups in tests that try to load extensions via command line
   when --enable-login-screen-apps was additionally specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions
==========

to

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified. This
happens in the following cases:
* On Chrome OS inside the user session after the browser has crashed at
  least once;
* In tests for Chrome OS that use stub or test user directly, without
  going through the login screen;
* In dev builds for Chrome OS running under Linux, when --login-manager
  is not passed via command line.

The effect of this bug is that:
1. Unwanted extensions were loaded, resulting in extra resources usage;
2. Log spam from signin_screen_policy_provider.cc with messages like
   "Denying load of Extension ...";
3. Error popups in tests that try to load extensions via command line
   when --enable-login-screen-apps was additionally specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions
==========

[emaxx, 2017-04-11 12:18:58]

emaxx@chromium.org changed reviewers:
+ yoshiki@chromium.org

[emaxx, 2017-04-11 12:18:59]

+yoshiki@: Please review
chrome/browser/chromeos/file_manager/file_manager_browsertest_base.cc.

[yoshiki, 2017-04-11 15:29:57]

lgtm

[emaxx, 2017-04-11 15:31:46]

The CQ bit was checked by emaxx@chromium.org

[commit-bot: I haz the power, 2017-04-11 15:32:35]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-11 17:07:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491924706743980,
"parent_rev": "9973ebb20992e56318308152e7b13e8f26fc087d", "commit_rev":
"095cadb4ac6310a09f9f76b81fc0ae8784323a87"}

[commit-bot: I haz the power, 2017-04-11 17:08:10]

Description was changed from

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified. This
happens in the following cases:
* On Chrome OS inside the user session after the browser has crashed at
  least once;
* In tests for Chrome OS that use stub or test user directly, without
  going through the login screen;
* In dev builds for Chrome OS running under Linux, when --login-manager
  is not passed via command line.

The effect of this bug is that:
1. Unwanted extensions were loaded, resulting in extra resources usage;
2. Log spam from signin_screen_policy_provider.cc with messages like
   "Denying load of Extension ...";
3. Error popups in tests that try to load extensions via command line
   when --enable-login-screen-apps was additionally specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions
==========

to

==========
Prevent loading of session extensions into the sign-in profile

The session extensions (like "Web Store" or "Bookmark Manager")
shouldn't be loaded into the original profile. However, they are loaded
currently as long as the --login-manager flag is not specified. This
happens in the following cases:
* On Chrome OS inside the user session after the browser has crashed at
  least once;
* In tests for Chrome OS that use stub or test user directly, without
  going through the login screen;
* In dev builds for Chrome OS running under Linux, when --login-manager
  is not passed via command line.

The effect of this bug is that:
1. Unwanted extensions were loaded, resulting in extra resources usage;
2. Log spam from signin_screen_policy_provider.cc with messages like
   "Denying load of Extension ...";
3. Error popups in tests that try to load extensions via command line
   when --enable-login-screen-apps was additionally specified.

This CL fixes this by extending the condition under which the session
extensions are skipped. Now it's also checking whether the profile is
the sign-in profile or the initial profile.

BUG=576464,674316
TEST=extended browser tests for file manager extensions

Review-Url: https://codereview.chromium.org/2807683003
Cr-Commit-Position: refs/heads/master@{#463661}
Committed:
https://chromium.googlesource.com/chromium/src/+/095cadb4ac6310a09f9f76b81fc0...
==========

[commit-bot: I haz the power, 2017-04-11 17:08:11]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/095cadb4ac6310a09f9f76b81fc0...