[service-manager] Make instance_group a Token

Changes Identity's instance group ID from a free-form string to a base::Token. Updates the 25-billion things which depend on this field. Some more details: - This removes kRootUserID in favor of a new C++ constant, kSystemInstanceGroup, a base::Token with a more approrpiate name given its purpose. - Also removes kInheritUserID. Because the instance group field is optional in an outgoing request Identity, nullopt is used to mean "inherit". - Not all conceptual references to "user ID" have been removed yet, since this CL is big enough as it is. For example, we still have a test-only service name "user_id", which no longer makes sense. Follow-up CLs will also change the instance ID field to a Token and clean up remaining conceptual references to "user ID". Bug: 895591 Change-Id: Idef13a269c6935123db68fa2621cecaca345974c Reviewed-on: https://chromium-review.googlesource.com/c/1325944 Commit-Queue: Ken Rockot <rockot@google.com> Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Eugene But <eugenebut@chromium.org> Reviewed-by: John Abd-El-Malek <jam@chromium.org> Cr-Commit-Position: refs/heads/master@{#607057}
FairyWorld · Nov 10, 2018 · da7edc6 · da7edc6
1 parent 8dfd9e2
commit da7edc6
Show file tree

Hide file tree

Showing 74 changed files with 489 additions and 415 deletions.
diff --git a/ash/assistant/assistant_controller.cc b/ash/assistant/assistant_controller.cc
@@ -321,15 +321,16 @@ void AssistantController::GetNavigableContentsFactory(
     return;
   }
 
-  const std::string& service_user_id = user_session->user_info->service_user_id;
-
-  if (service_user_id.empty()) {
-    LOG(ERROR) << "Unable to retrieve service user id.";
+  const base::Optional<base::Token>& service_instance_group =
+      user_session->user_info->service_instance_group;
+  if (!service_instance_group) {
+    LOG(ERROR) << "Unable to retrieve service instance group.";
     return;
   }
 
   Shell::Get()->connector()->BindInterface(
-      service_manager::Identity(content::mojom::kServiceName, service_user_id),
+      service_manager::Identity(content::mojom::kServiceName,
+                                service_instance_group),
       std::move(request));
 }
 

diff --git a/ash/multi_device_setup/multi_device_notification_presenter.cc b/ash/multi_device_setup/multi_device_notification_presenter.cc
@@ -226,15 +226,17 @@ void MultiDeviceNotificationPresenter::ObserveMultiDeviceSetupIfPossible() {
   if (!user_session)
     return;
 
-  std::string service_user_id = user_session->user_info->service_user_id;
+  base::Optional<base::Token> service_instance_group =
+      user_session->user_info->service_instance_group;
 
-  // Cannot proceed if there is no service user ID.
-  if (service_user_id.empty())
+  // Cannot proceed if there is no known service instance group.
+  if (!service_instance_group)
     return;
 
   connector_->BindInterface(
       service_manager::Identity(
-          chromeos::multidevice_setup::mojom::kServiceName, service_user_id),
+          chromeos::multidevice_setup::mojom::kServiceName,
+          service_instance_group),
       &multidevice_setup_ptr_);
 
   // Add this object as the delegate of the MultiDeviceSetup Service.

diff --git a/ash/multi_device_setup/multi_device_notification_presenter_unittest.cc b/ash/multi_device_setup/multi_device_notification_presenter_unittest.cc
@@ -16,6 +16,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/token.h"
 #include "chromeos/services/multidevice_setup/public/cpp/fake_multidevice_setup.h"
 #include "chromeos/services/multidevice_setup/public/mojom/constants.mojom.h"
 #include "chromeos/services/multidevice_setup/public/mojom/multidevice_setup.mojom.h"
@@ -32,8 +33,8 @@ namespace {
 const char kTestUserEmail[] = "test@example.com";
 const char kTestHostDeviceName[] = "Test Device";
 
-// Note: Must be formatted as a GUID.
-const char kTestServiceUserId[] = "01234567-89ab-cdef-0123-456789abcdef";
+const base::Token kTestServiceInstanceGroup{0x0123456789abcdefull,
+                                            0xfedcba9876543210ull};
 
 class TestMessageCenter : public message_center::FakeMessageCenter {
  public:
@@ -137,7 +138,7 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
     test_api.OverrideBinderForTesting(
         service_manager::Identity(
             chromeos::multidevice_setup::mojom::kServiceName,
-            kTestServiceUserId),
+            kTestServiceInstanceGroup),
         chromeos::multidevice_setup::mojom::MultiDeviceSetup::Name_,
         base::BindRepeating(
             &chromeos::multidevice_setup::FakeMultiDeviceSetup::BindHandle,
@@ -163,7 +164,7 @@ class MultiDeviceNotificationPresenterTest : public NoSessionAshTestBase {
     test_session_client->AddUserSession(
         kTestUserEmail, user_manager::USER_TYPE_REGULAR,
         true /* enable_settings */, true /* provide_pref_service */,
-        false /* is_new_profile */, kTestServiceUserId);
+        false /* is_new_profile */, kTestServiceInstanceGroup);
     test_session_client->SetSessionState(session_manager::SessionState::ACTIVE);
     test_session_client->SwitchActiveUser(
         AccountId::FromUserEmail(kTestUserEmail));

diff --git a/ash/public/interfaces/user_info.mojom b/ash/public/interfaces/user_info.mojom
@@ -5,6 +5,7 @@
 module ash.mojom;
 
 import "components/account_id/interfaces/account_id.mojom";
+import "mojo/public/mojom/base/token.mojom";
 import "ui/gfx/image/mojo/image.mojom";
 
 // Matches user_manager::UserType.
@@ -49,7 +50,7 @@ struct UserAvatar {
 struct UserInfo {
   UserType type;
   signin.mojom.AccountId account_id;
-  string service_user_id;
+  mojo_base.mojom.Token? service_instance_group;
   string display_name;
   string display_email;
   UserAvatar avatar;

diff --git a/ash/session/test_session_controller_client.cc b/ash/session/test_session_controller_client.cc
@@ -138,7 +138,7 @@ void TestSessionControllerClient::AddUserSession(
     bool enable_settings,
     bool provide_pref_service,
     bool is_new_profile,
-    const std::string& service_user_id) {
+    const base::Optional<base::Token>& service_instance_group) {
   auto account_id = AccountId::FromUserEmail(
       use_lower_case_user_id_ ? GetUserIdFromEmail(display_email)
                               : display_email);
@@ -148,7 +148,7 @@ void TestSessionControllerClient::AddUserSession(
   session->user_info->avatar = mojom::UserAvatar::New();
   session->user_info->type = user_type;
   session->user_info->account_id = account_id;
-  session->user_info->service_user_id = service_user_id;
+  session->user_info->service_instance_group = service_instance_group;
   session->user_info->display_name = "Über tray Über tray Über tray Über tray";
   session->user_info->display_email = display_email;
   session->user_info->is_ephemeral = false;

diff --git a/ash/session/test_session_controller_client.h b/ash/session/test_session_controller_client.h
@@ -11,6 +11,8 @@
 
 #include "ash/public/interfaces/session_controller.mojom.h"
 #include "base/macros.h"
+#include "base/optional.h"
+#include "base/token.h"
 #include "components/user_manager/user_type.h"
 #include "mojo/public/cpp/bindings/binding.h"
 
@@ -70,7 +72,8 @@ class TestSessionControllerClient : public ash::mojom::SessionControllerClient {
       bool enable_settings = true,
       bool provide_pref_service = true,
       bool is_new_profile = false,
-      const std::string& service_user_id = std::string());
+      const base::Optional<base::Token>& service_instance_group =
+          base::nullopt);
 
   // Creates a test PrefService and associates it with the user.
   void ProvidePrefServiceForUser(const AccountId& account_id);

diff --git a/ash/test/ash_test_helper.cc b/ash/test/ash_test_helper.cc
@@ -25,6 +25,7 @@
 #include "base/guid.h"
 #include "base/run_loop.h"
 #include "base/strings/string_split.h"
+#include "base/token.h"
 #include "chromeos/audio/cras_audio_handler.h"
 #include "chromeos/cryptohome/system_salt_getter.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
@@ -106,7 +107,7 @@ class TestGpuInterfaceProvider : public ws::GpuInterfaceProvider {
 // TODO(sky): refactor and move to services.
 class TestConnector : public service_manager::mojom::Connector {
  public:
-  TestConnector() : test_user_id_(base::GenerateGUID()) {}
+  TestConnector() : test_instance_group_(base::Token::CreateRandom()) {}
 
   ~TestConnector() override = default;
 
@@ -116,7 +117,7 @@ class TestConnector : public service_manager::mojom::Connector {
 
   void Start() {
     service_ptr_->OnStart(
-        service_manager::Identity("TestConnectorFactory", test_user_id_),
+        service_manager::Identity("TestConnectorFactory", test_instance_group_),
         base::BindOnce(&TestConnector::OnStartCallback,
                        base::Unretained(this)));
   }
@@ -146,7 +147,7 @@ class TestConnector : public service_manager::mojom::Connector {
     (*service_ptr)
         ->OnBindInterface(service_manager::BindSourceInfo(
                               service_manager::Identity("TestConnectorFactory",
-                                                        test_user_id_),
+                                                        test_instance_group_),
                               service_manager::CapabilitySet()),
                           interface_name, std::move(interface_pipe),
                           base::DoNothing());
@@ -184,7 +185,7 @@ class TestConnector : public service_manager::mojom::Connector {
     NOTREACHED();
   }
 
-  const std::string test_user_id_;
+  const base::Token test_instance_group_;
   mojo::BindingSet<service_manager::mojom::Connector> bindings_;
   service_manager::mojom::ServicePtr service_ptr_;
 

diff --git a/chrome/browser/prefs/profile_pref_store_manager_unittest.cc b/chrome/browser/prefs/profile_pref_store_manager_unittest.cc
@@ -38,6 +38,7 @@
 #include "services/preferences/public/cpp/tracked/pref_names.h"
 #include "services/preferences/public/mojom/preferences.mojom.h"
 #include "services/service_manager/public/cpp/connector.h"
+#include "services/service_manager/public/cpp/constants.h"
 #include "services/service_manager/public/cpp/service_context.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -356,7 +357,7 @@ class ProfilePrefStoreManagerTest : public testing::Test,
                      mojo::ScopedMessagePipeHandle handle) {
     service_manager::BindSourceInfo source(
         service_manager::Identity(content::mojom::kBrowserServiceName,
-                                  service_manager::mojom::kRootUserID),
+                                  service_manager::kSystemInstanceGroup),
         service_manager::CapabilitySet());
     static_cast<service_manager::mojom::Service*>(pref_service_context_.get())
         ->OnBindInterface(source, interface_name, std::move(handle),

diff --git a/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc b/chrome/browser/spellchecker/spell_check_host_chrome_impl.cc
@@ -81,8 +81,8 @@ void SpellCheckHostChromeImpl::CallSpellingService(
   // a response is received (including an error) from the remote Spelling
   // service, calls CallSpellingServiceDone.
   content::BrowserContext* context =
-      content::BrowserContext::GetBrowserContextForServiceUserId(
-          renderer_identity_.instance_group());
+      content::BrowserContext::GetBrowserContextForServiceInstanceGroup(
+          *renderer_identity_.instance_group());
   client_.RequestTextCheck(
       context, SpellingServiceClient::SPELLCHECK, text,
       base::BindOnce(&SpellCheckHostChromeImpl::CallSpellingServiceDone,

diff --git a/chrome/browser/spellchecker/spell_check_host_chrome_impl_mac.cc b/chrome/browser/spellchecker/spell_check_host_chrome_impl_mac.cc
@@ -137,8 +137,8 @@ void SpellingRequest::RequestRemoteCheck(
     SpellingServiceClient* client,
     const service_manager::Identity& renderer_identity) {
   BrowserContext* context =
-      content::BrowserContext::GetBrowserContextForServiceUserId(
-          renderer_identity.instance_group());
+      content::BrowserContext::GetBrowserContextForServiceInstanceGroup(
+          *renderer_identity.instance_group());
 
   // |this| may be gone at callback invocation if the owner has been removed.
   client->RequestTextCheck(

diff --git a/chrome/browser/spellchecker/spellcheck_factory.cc b/chrome/browser/spellchecker/spellcheck_factory.cc
@@ -28,8 +28,8 @@ SpellcheckService* SpellcheckServiceFactory::GetForContext(
 SpellcheckService* SpellcheckServiceFactory::GetForRenderer(
     const service_manager::Identity& renderer_identity) {
   content::BrowserContext* context =
-      content::BrowserContext::GetBrowserContextForServiceUserId(
-          renderer_identity.instance_group());
+      content::BrowserContext::GetBrowserContextForServiceInstanceGroup(
+          *renderer_identity.instance_group());
   if (!context)
     return nullptr;
   return GetForContext(context);

diff --git a/chrome/browser/spellchecker/spellcheck_service.cc b/chrome/browser/spellchecker/spellcheck_service.cc
@@ -182,8 +182,8 @@ void SpellcheckService::InitForRenderer(
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   content::BrowserContext* context =
-      content::BrowserContext::GetBrowserContextForServiceUserId(
-          renderer_identity.instance_group());
+      content::BrowserContext::GetBrowserContextForServiceInstanceGroup(
+          *renderer_identity.instance_group());
   if (SpellcheckServiceFactory::GetForContext(context) != this)
     return;
 

diff --git a/chrome/browser/ui/ash/session_controller_client.cc b/chrome/browser/ui/ash/session_controller_client.cc
@@ -104,8 +104,8 @@ ash::mojom::UserSessionPtr UserToUserSession(const User& user) {
   session->user_info->is_device_owner =
       owner_id.is_valid() && owner_id == user.GetAccountId();
   if (profile) {
-    session->user_info->service_user_id =
-        content::BrowserContext::GetServiceUserIdFor(profile);
+    session->user_info->service_instance_group =
+        content::BrowserContext::GetServiceInstanceGroupFor(profile);
     session->user_info->is_new_profile = profile->IsNewProfile();
   }
 

diff --git a/chrome/browser/ui/ash/session_controller_client_unittest.cc b/chrome/browser/ui/ash/session_controller_client_unittest.cc
@@ -498,8 +498,9 @@ TEST_F(SessionControllerClientTest, SendUserSession) {
   // User session was sent.
   EXPECT_EQ(1, session_controller.update_user_session_count());
   ASSERT_TRUE(session_controller.last_user_session());
-  EXPECT_EQ(content::BrowserContext::GetServiceUserIdFor(user_profile),
-            session_controller.last_user_session()->user_info->service_user_id);
+  EXPECT_EQ(content::BrowserContext::GetServiceInstanceGroupFor(user_profile),
+            session_controller.last_user_session()
+                ->user_info->service_instance_group.value());
 
   // Simulate a request for an update where nothing changed.
   client.SendUserSession(*user_manager()->GetLoggedInUsers()[0]);

diff --git a/chrome/service/service_utility_process_host.cc b/chrome/service/service_utility_process_host.cc
@@ -52,7 +52,7 @@
 #include "services/service_manager/embedder/switches.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/cpp/connector.h"
-#include "services/service_manager/public/mojom/constants.mojom.h"
+#include "services/service_manager/public/cpp/constants.h"
 #include "services/service_manager/public/mojom/service.mojom.h"
 #include "services/service_manager/runner/host/service_process_launcher.h"
 #include "services/service_manager/runner/host/service_process_launcher_factory.h"
@@ -373,7 +373,7 @@ bool ServiceUtilityProcessHost::StartProcess(bool sandbox) {
   service_manager::mojom::PIDReceiverPtr pid_receiver;
   service_manager_->RegisterService(
       service_manager::Identity(content::mojom::kBrowserServiceName,
-                                service_manager::mojom::kRootUserID),
+                                service_manager::kSystemInstanceGroup),
       std::move(browser_proxy), mojo::MakeRequest(&pid_receiver));
   pid_receiver->SetPID(base::GetCurrentProcId());
   pid_receiver.reset();
@@ -384,7 +384,7 @@ bool ServiceUtilityProcessHost::StartProcess(bool sandbox) {
       mojo_invitation_.AttachMessagePipe(mojo_bootstrap_token), 0u));
   service_manager_->RegisterService(
       service_manager::Identity(content::mojom::kUtilityServiceName,
-                                service_manager::mojom::kRootUserID),
+                                service_manager::kSystemInstanceGroup),
       std::move(utility_service), mojo::MakeRequest(&pid_receiver));
   pid_receiver->SetPID(base::GetCurrentProcId());
 

diff --git a/content/browser/browser_child_process_host_impl.cc b/content/browser/browser_child_process_host_impl.cc
@@ -169,7 +169,7 @@ BrowserChildProcessHostImpl::BrowserChildProcessHostImpl(
   if (!service_name.empty()) {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
     service_manager::Identity child_identity(
-        service_name, service_manager::mojom::kInheritUserID,
+        service_name, base::nullopt /* instance_group */,
         base::StringPrintf("%d", data_.id));
     child_connection_.reset(
         new ChildConnection(child_identity, &mojo_invitation_,