Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
[M113] Cherry-pick a fix for shutdown crash
The shutdown crash is caused by the code that removes the extension disable reason from the user who is not supervised any more. The disable reason is incorrectly removed for supervised users during the SupervisedUserService shutdown and cleanup. The changes applied to the original cl in order to compile on M113: 1) Use Profile:IsChild() instead of SuperviseUserService::IsSubjectToParentalControls() to check whether user is supervised, because later is not available in M113. 3) Use prefs name from chrome/common, because kSupervisedUserId was not moved yet. Tested manually on M113 branch. Confirmed that crash is not being reported anymore. --------------------------------------------------- Reland "Enable supervised_user_extension_browsertest.cc and introduce the SupervisionMixin that replaces LoggedInUserMixin." This is a reland of commit 101042b The diff against reverted change is that the preferences regarding the supervised user profile are set directly, rather than TestingProfile. Original change's description: > Enable supervised_user_extension_browsertest.cc and introduce the SupervisedUserMixin that replaces LoggedInUserMixin. > > Bug: b:276874936, 1218633 > Change-Id: Iea0a1255bf20265ab9411227429138f3433bf186 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4427809 > Reviewed-by: Colin Blundell <blundell@chromium.org> > Commit-Queue: Tomek Jurkiewicz <tju@google.com> > Reviewed-by: Nohemi Fernandez <fernandex@chromium.org> > Cr-Commit-Position: refs/heads/main@{#1137145} (cherry picked from commit 19f06e4) Bug: 1435569 Change-Id: I067f86bd989bca0da54a64b5c0dee0478c883679 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4492183 Commit-Queue: Tomek Jurkiewicz <tju@google.com> Reviewed-by: Finnur Thorarinsson <finnur@chromium.org> Reviewed-by: Nohemi Fernandez <fernandex@chromium.org> Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#1138312} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4533335 Commit-Queue: Aga Wronska <agawronska@chromium.org> Reviewed-by: Courtney Wong <courtneywong@chromium.org> Owners-Override: Prudhvikumar Bommana <pbommana@google.com> Cr-Commit-Position: refs/branch-heads/5672@{#1222} Cr-Branched-From: 5f2a724-refs/heads/main@{#1121455}
- Loading branch information
Aga Wronska
authored and
Chromium LUCI CQ
committed
May 17, 2023
1 parent
310cab0
commit 67bd471
Showing
6 changed files
with
264 additions
and
53 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| file://components/supervised_user/OWNERS |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| // 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/test/supervised_user/supervision_mixin.h" | ||
|
|
||
| #include <memory> | ||
|
|
||
| #include "base/check.h" | ||
| #include "base/functional/bind.h" | ||
| #include "base/strings/string_piece.h" | ||
| #include "chrome/browser/profiles/profile.h" | ||
| #include "chrome/browser/signin/identity_test_environment_profile_adaptor.h" | ||
| #include "chrome/browser/ui/browser.h" | ||
| #include "chrome/common/pref_names.h" | ||
| #include "chrome/test/base/mixin_based_in_process_browser_test.h" | ||
| #include "chrome/test/base/testing_profile.h" | ||
| #include "components/keyed_service/content/browser_context_dependency_manager.h" | ||
| #include "components/signin/public/base/consent_level.h" | ||
| #include "components/signin/public/identity_manager/account_info.h" | ||
| #include "components/signin/public/identity_manager/identity_manager.h" | ||
| #include "components/signin/public/identity_manager/identity_test_utils.h" | ||
| #include "components/signin/public/identity_manager/tribool.h" | ||
| #include "components/supervised_user/core/common/supervised_user_constants.h" | ||
| #include "content/public/browser/browser_context.h" | ||
| #include "content/public/test/browser_test_utils.h" | ||
| #include "net/dns/mock_host_resolver.h" | ||
|
|
||
| namespace supervised_user { | ||
|
|
||
| namespace { | ||
| void OnWillCreateBrowserContextServices(content::BrowserContext* context) { | ||
| // Sets all required testing factories to have control over identity | ||
| // environment during test. Effectively, this substitutes the real identity | ||
| // environment with identity test environment, taking care to fulfill all | ||
| // required dependencies. | ||
| IdentityTestEnvironmentProfileAdaptor:: | ||
| SetIdentityTestEnvironmentFactoriesOnBrowserContext(context); | ||
| } | ||
|
|
||
| bool IdentityManagerAlreadyHasPrimaryAccount( | ||
| signin::IdentityManager* identity_manager, | ||
| base::StringPiece email, | ||
| signin::ConsentLevel consent_level) { | ||
| if (!identity_manager->HasPrimaryAccount(consent_level)) { | ||
| return false; | ||
| } | ||
| CoreAccountInfo account_info = | ||
| identity_manager->GetPrimaryAccountInfo(consent_level); | ||
| return account_info.email == email; | ||
| } | ||
|
|
||
| void SetIsSupervisedProfile(Profile* profile, bool is_supervised_profile) { | ||
| if (is_supervised_profile) { | ||
| profile->GetPrefs()->SetString(prefs::kSupervisedUserId, | ||
| supervised_user::kChildAccountSUID); | ||
| } else { | ||
| profile->GetPrefs()->ClearPref(prefs::kSupervisedUserId); | ||
| } | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| SupervisionMixin::SupervisionMixin( | ||
| InProcessBrowserTestMixinHost& test_mixin_host, | ||
| InProcessBrowserTest* test_base) | ||
| : SupervisionMixin(test_mixin_host, | ||
| test_base, | ||
| SupervisionMixin::Options()) {} | ||
|
|
||
| SupervisionMixin::SupervisionMixin( | ||
| InProcessBrowserTestMixinHost& test_mixin_host, | ||
| InProcessBrowserTest* test_base, | ||
| const Options& options) | ||
| : InProcessBrowserTestMixin(&test_mixin_host), | ||
| test_base_(test_base), | ||
| fake_gaia_mixin_(&test_mixin_host), | ||
| consent_level_(options.consent_level), | ||
| email_(options.email), | ||
| account_type_(options.account_type) {} | ||
|
|
||
| SupervisionMixin::~SupervisionMixin() = default; | ||
|
|
||
| void SupervisionMixin::SetUpInProcessBrowserTestFixture() { | ||
| subscription_ = | ||
| BrowserContextDependencyManager::GetInstance() | ||
| ->RegisterCreateServicesCallbackForTesting( | ||
| base::BindRepeating(&OnWillCreateBrowserContextServices)); | ||
| } | ||
|
|
||
| void SupervisionMixin::SetUpOnMainThread() { | ||
| SetUpIdentityTestEnvironment(); | ||
| LogInUser(); | ||
| SetUpTestServer(); | ||
| } | ||
|
|
||
| void SupervisionMixin::SetUpTestServer() { | ||
| // By default, browser tests block anything that doesn't go to localhost, so | ||
| // account.google.com requests would never reach fake GAIA server without | ||
| // this. | ||
| test_base_->host_resolver()->AddRule("*", "127.0.0.1"); | ||
| } | ||
|
|
||
| void SupervisionMixin::SetUpIdentityTestEnvironment() { | ||
| adaptor_ = std::make_unique<IdentityTestEnvironmentProfileAdaptor>( | ||
| test_base_->browser()->profile()); | ||
| } | ||
|
|
||
| void SupervisionMixin::LogInUser() { | ||
| if (!IdentityManagerAlreadyHasPrimaryAccount( | ||
| GetIdentityTestEnvironment()->identity_manager(), email_, | ||
| consent_level_)) { | ||
| // PRE_ tests intentionally leave accounts that are picked up by subsequent | ||
| // test runs. | ||
| AccountInfo account_info = | ||
| GetIdentityTestEnvironment()->MakePrimaryAccountAvailable( | ||
| email_, consent_level_); | ||
| CHECK(!account_info.account_id.empty()); | ||
| } | ||
|
|
||
| GetIdentityTestEnvironment()->SetRefreshTokenForPrimaryAccount(); | ||
| GetIdentityTestEnvironment()->SetAutomaticIssueOfAccessTokens(true); | ||
|
|
||
| SetIsSupervisedProfile( | ||
| test_base_->browser()->profile(), | ||
| /*is_supervised_profile=*/account_type_ == AccountType::kSupervised); | ||
| } | ||
|
|
||
| signin::IdentityTestEnvironment* | ||
| SupervisionMixin::GetIdentityTestEnvironment() { | ||
| CHECK(adaptor_->identity_test_env()) | ||
| << "Do not use before the environment is set up."; | ||
| return adaptor_->identity_test_env(); | ||
| } | ||
|
|
||
| } // namespace supervised_user |
Oops, something went wrong.