Updater: Server should only listen to one XPC connection per process.

Bug: 1112522
Change-Id: Idcae3a8103e817fd1c70394a21680df4e6621163
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2387431
Commit-Queue: Mila Green <milagreen@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803596}

chrome/updater/app/server/mac/server.mm

@@ -8,6 +8,7 @@
 #include <xpc/xpc.h>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/mac/foundation_util.h"
 #include "base/mac/scoped_nsobject.h"
@@ -19,6 +20,7 @@
 #import "chrome/updater/app/server/mac/app_server.h"
 #include "chrome/updater/app/server/mac/service_delegate.h"
 #include "chrome/updater/configurator.h"
+#include "chrome/updater/constants.h"
 #include "chrome/updater/control_service_in_process.h"
 #include "chrome/updater/mac/setup/setup.h"
 #import "chrome/updater/mac/xpc_service_names.h"
@@ -43,32 +45,49 @@ void AppServerMac::Uninitialize() {
 
 void AppServerMac::ActiveDuty() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-
-  @autoreleasepool {
-    // Sets up a listener and delegate for the CRUUpdateChecking XPC
-    // connection
-    update_check_delegate_.reset([[CRUUpdateCheckServiceXPCDelegate alloc]
-        initWithUpdateService:base::MakeRefCounted<UpdateServiceInProcess>(
-                                  config_)
-                    appServer:scoped_refptr<AppServerMac>(this)]);
-
-    update_check_listener_.reset([[NSXPCListener alloc]
-        initWithMachServiceName:GetServiceMachName().get()]);
-    update_check_listener_.get().delegate = update_check_delegate_.get();
-
-    [update_check_listener_ resume];
-
-    // Sets up a listener and delegate for the CRUControlling XPC connection
-    control_service_delegate_.reset([[CRUControlServiceXPCDelegate alloc]
-        initWithControlService:base::MakeRefCounted<ControlServiceInProcess>(
-                                   config_)
-                     appServer:scoped_refptr<AppServerMac>(this)]);
-
-    control_service_listener_.reset([[NSXPCListener alloc]
-        initWithMachServiceName:GetVersionedServiceMachName().get()]);
-    control_service_listener_.get().delegate = control_service_delegate_.get();
-
-    [control_service_listener_ resume];
+  const base::CommandLine& command_line =
+      *base::CommandLine::ForCurrentProcess();
+  if (!command_line.HasSwitch(kServerServiceSwitch)) {
+    LOG(ERROR) << "Command line is missing " << kServerServiceSwitch
+               << " switch.";
+    return;
+  }
+  std::string service = command_line.GetSwitchValueASCII(kServerServiceSwitch);
+
+  if (service == kServerControlServiceSwitchValue) {
+    @autoreleasepool {
+      // Sets up a listener and delegate for the CRUControlling XPC connection.
+      control_service_delegate_.reset([[CRUControlServiceXPCDelegate alloc]
+          initWithControlService:base::MakeRefCounted<ControlServiceInProcess>(
+                                     config_)
+                       appServer:scoped_refptr<AppServerMac>(this)]);
+
+      control_service_listener_.reset([[NSXPCListener alloc]
+          initWithMachServiceName:GetVersionedServiceMachName().get()]);
+      control_service_listener_.get().delegate =
+          control_service_delegate_.get();
+
+      [control_service_listener_ resume];
+    }
+  } else if (service == kServerUpdateServiceSwitchValue) {
+    @autoreleasepool {
+      // Sets up a listener and delegate for the CRUUpdateChecking XPC
+      // connection.
+      update_check_delegate_.reset([[CRUUpdateCheckServiceXPCDelegate alloc]
+          initWithUpdateService:base::MakeRefCounted<UpdateServiceInProcess>(
+                                    config_)
+                      appServer:scoped_refptr<AppServerMac>(this)]);
+
+      update_check_listener_.reset([[NSXPCListener alloc]
+          initWithMachServiceName:GetServiceMachName().get()]);
+      update_check_listener_.get().delegate = update_check_delegate_.get();
+
+      [update_check_listener_ resume];
+    }
+  } else {
+    LOG(ERROR) << "Unexpected value of command line switch "
+               << kServerServiceSwitch << ": " << service;
+    return;
   }
 }
 

chrome/updater/constants.cc

@@ -30,6 +30,10 @@ const char kAppIdSwitch[] = "app-id";
 const char kAppVersionSwitch[] = "app-version";
 const char kWakeSwitch[] = "wake";
 
+const char kServerServiceSwitch[] = "service";
+const char kServerControlServiceSwitchValue[] = "control";
+const char kServerUpdateServiceSwitchValue[] = "update";
+
 #if defined(OS_WIN)
 const char kInstallFromOutDir[] = "install-from-out-dir";
 #endif  // OS_WIN

chrome/updater/constants.h

@@ -38,6 +38,13 @@ extern const char kNullVersion[];
 //   (say, read-only) functionality for that same set of interfaces.
 extern const char kServerSwitch[];
 
+// This switch specifies the XPC service the server registers to listen to.
+extern const char kServerServiceSwitch[];
+
+// Valid values for the kServerServiceSwitch.
+extern const char kServerControlServiceSwitchValue[];
+extern const char kServerUpdateServiceSwitchValue[];
+
 // This switch starts the COM service. This switch is invoked by the Service
 // Manager when CoCreate is called on one of several CLSIDs that the server
 // supports.

chrome/updater/mac/setup/setup.mm

@@ -33,6 +33,8 @@ namespace updater {
 
 namespace {
 
+constexpr char kLoggingModuleSwitchValue[] = "*/updater/*=2";
+
 #pragma mark Helpers
 const base::FilePath GetUpdateFolderName() {
   return base::FilePath(COMPANY_SHORTNAME_STRING)
@@ -113,6 +115,11 @@ NSString* MakeProgramArgument(const char* argument) {
   return base::SysUTF8ToNSString(base::StrCat({"--", argument}));
 }
 
+NSString* MakeProgramArgumentWithValue(const char* argument,
+                                       const char* value) {
+  return base::SysUTF8ToNSString(base::StrCat({"--", argument, "=", value}));
+}
+
 base::ScopedCFTypeRef<CFDictionaryRef> CreateServiceLaunchdPlist(
     const base::FilePath& updater_path) {
   // See the man page for launchd.plist.
@@ -121,7 +128,10 @@ base::ScopedCFTypeRef<CFDictionaryRef> CreateServiceLaunchdPlist(
     @LAUNCH_JOBKEY_PROGRAMARGUMENTS : @[
       base::SysUTF8ToNSString(updater_path.value()),
       MakeProgramArgument(kServerSwitch),
-      @"--vmodule=*/updater/*=2",
+      MakeProgramArgumentWithValue(kServerServiceSwitch,
+                                   kServerUpdateServiceSwitchValue),
+      MakeProgramArgumentWithValue(kLoggingModuleSwitch,
+                                   kLoggingModuleSwitchValue),
     ],
     @LAUNCH_JOBKEY_MACHSERVICES : @{GetServiceMachName() : @YES},
     @LAUNCH_JOBKEY_ABANDONPROCESSGROUP : @NO,
@@ -166,7 +176,10 @@ base::ScopedCFTypeRef<CFDictionaryRef> CreateControlLaunchdPlist(
     @LAUNCH_JOBKEY_PROGRAMARGUMENTS : @[
       base::SysUTF8ToNSString(updater_path.value()),
       MakeProgramArgument(kServerSwitch),
-      @"--vmodule=*/updater/*=2",
+      MakeProgramArgumentWithValue(kServerServiceSwitch,
+                                   kServerControlServiceSwitchValue),
+      MakeProgramArgumentWithValue(kLoggingModuleSwitch,
+                                   kLoggingModuleSwitchValue),
     ],
     @LAUNCH_JOBKEY_MACHSERVICES : @{GetVersionedServiceMachName() : @YES},
     @LAUNCH_JOBKEY_ABANDONPROCESSGROUP : @NO,