Bug 1518639: Move remote client call to after we have a profile. r=jimm
☠☠ backed out by 3d8dd3615c45 ☠ ☠
authorDave Townsend <dtownsend@oxymoronical.com>
Thu, 31 Jan 2019 17:06:00 -0800
changeset 520627 967993505a3d00a79cd81dccf66ffa0612a58ad4
parent 520626 fc466857ab39e3d2371f13ddae553c921fb727d2
child 520628 73ca9a68d7714e9bfc4d4ed59c8dda5ffdc3cd86
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjimm
bugs1518639
milestone67.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1518639: Move remote client call to after we have a profile. r=jimm Makes it so we always know which profile we want to remote the command line to. Differential Revision: https://phabricator.services.mozilla.com/D19073
toolkit/components/remote/nsDBusRemoteClient.cpp
toolkit/components/remote/nsRemoteService.cpp
toolkit/components/remote/nsRemoteService.h
toolkit/components/remote/nsXRemoteClient.cpp
toolkit/components/remote/nsXRemoteServer.cpp
toolkit/xre/nsAppRunner.cpp
--- a/toolkit/components/remote/nsDBusRemoteClient.cpp
+++ b/toolkit/components/remote/nsDBusRemoteClient.cpp
@@ -72,89 +72,48 @@ nsresult nsDBusRemoteClient::SendCommand
           ("DoSendDBusCommandLine returning 0x%" PRIx32 "\n",
            static_cast<uint32_t>(rv)));
   return rv;
 }
 
 bool nsDBusRemoteClient::GetRemoteDestinationName(const char *aProgram,
                                                   const char *aProfile,
                                                   nsCString &aDestinationName) {
-  if (!aProfile || aProfile[0] == '\0') {
-    // We don't have a profile name - search for active mozilla instances.
-    RefPtr<DBusMessage> msg =
-        already_AddRefed<DBusMessage>(dbus_message_new_method_call(
-            "org.freedesktop.DBus", "/org/freedesktop/DBus",
-            "org.freedesktop.DBus", "ListNames"));
-    if (!msg) {
-      return false;
-    }
+  // We have a profile name - just create the destination.
+  // D-Bus names can contain only [a-z][A-Z][0-9]_
+  // characters so adjust the profile string properly.
+  nsAutoCString profileName;
+  nsresult rv = mozilla::Base64Encode(nsAutoCString(aProfile), profileName);
+  NS_ENSURE_SUCCESS(rv, false);
+  profileName.ReplaceChar("+/=", '_');
+
+  aDestinationName =
+      nsPrintfCString("org.mozilla.%s.%s", aProgram, profileName.get());
+  if (aDestinationName.Length() > DBUS_MAXIMUM_NAME_LENGTH)
+    aDestinationName.Truncate(DBUS_MAXIMUM_NAME_LENGTH);
 
-    // send message and get a handle for a reply
-    RefPtr<DBusMessage> reply =
-        already_AddRefed<DBusMessage>(dbus_connection_send_with_reply_and_block(
-            mConnection, msg, -1, nullptr));
-    if (!reply) {
-      return false;
-    }
+  static auto sDBusValidateBusName =
+      (bool (*)(const char *, DBusError *))dlsym(RTLD_DEFAULT,
+                                                  "dbus_validate_bus_name");
+  if (!sDBusValidateBusName) {
+    return false;
+  }
 
-    char **interfaces;
-    dbus_int32_t interfaceNums;
-    if (!dbus_message_get_args(reply, nullptr, DBUS_TYPE_ARRAY,
-                               DBUS_TYPE_STRING, &interfaces, &interfaceNums,
-                               DBUS_TYPE_INVALID)) {
+  if (!sDBusValidateBusName(aDestinationName.get(), nullptr)) {
+    // We don't have a valid busName yet - try to create a default one.
+    aDestinationName =
+        nsPrintfCString("org.mozilla.%s.%s", aProgram, "default");
+    if (!sDBusValidateBusName(aDestinationName.get(), nullptr)) {
+      // We failed completelly to get a valid bus name - just quit
+      // to prevent crash at dbus_bus_request_name().
       return false;
     }
-
-    nsAutoCString destinationTemplate;
-    destinationTemplate = nsPrintfCString("org.mozilla.%s", aProgram);
-
-    aDestinationName.SetLength(0);
-    for (int i = 0; i < interfaceNums; i++) {
-      if (strstr(interfaces[i], destinationTemplate.get())) {
-        aDestinationName = interfaces[i];
-        break;
-      }
-    }
-    dbus_free_string_array(interfaces);
-
-    return (!aDestinationName.IsEmpty());
-  } else {
-    // We have a profile name - just create the destination.
-    // D-Bus names can contain only [a-z][A-Z][0-9]_
-    // characters so adjust the profile string properly.
-    nsAutoCString profileName;
-    nsresult rv = mozilla::Base64Encode(nsAutoCString(aProfile), profileName);
-    NS_ENSURE_SUCCESS(rv, false);
-    profileName.ReplaceChar("+/=", '_');
+  }
 
-    aDestinationName =
-        nsPrintfCString("org.mozilla.%s.%s", aProgram, profileName.get());
-    if (aDestinationName.Length() > DBUS_MAXIMUM_NAME_LENGTH)
-      aDestinationName.Truncate(DBUS_MAXIMUM_NAME_LENGTH);
-
-    static auto sDBusValidateBusName =
-        (bool (*)(const char *, DBusError *))dlsym(RTLD_DEFAULT,
-                                                   "dbus_validate_bus_name");
-    if (!sDBusValidateBusName) {
-      return false;
-    }
-
-    if (!sDBusValidateBusName(aDestinationName.get(), nullptr)) {
-      // We don't have a valid busName yet - try to create a default one.
-      aDestinationName =
-          nsPrintfCString("org.mozilla.%s.%s", aProgram, "default");
-      if (!sDBusValidateBusName(aDestinationName.get(), nullptr)) {
-        // We failed completelly to get a valid bus name - just quit
-        // to prevent crash at dbus_bus_request_name().
-        return false;
-      }
-    }
-
-    return true;
-  }
+  return true;
 }
 
 nsresult nsDBusRemoteClient::DoSendDBusCommandLine(const char *aProgram,
                                                    const char *aProfile,
                                                    const char *aBuffer,
                                                    int aLength) {
   nsAutoCString destinationName;
   if (!GetRemoteDestinationName(aProgram, aProfile, destinationName))
--- a/toolkit/components/remote/nsRemoteService.cpp
+++ b/toolkit/components/remote/nsRemoteService.cpp
@@ -38,16 +38,20 @@ extern char** gArgv;
 using namespace mozilla;
 
 NS_IMPL_ISUPPORTS(nsRemoteService, nsIObserver)
 
 nsRemoteService::nsRemoteService(const char* aProgram) : mProgram(aProgram) {
   ToLowerCase(mProgram);
 }
 
+void nsRemoteService::SetProfile(nsACString& aProfile) {
+  mProfile = aProfile;
+}
+
 void nsRemoteService::LockStartup() {
   nsCOMPtr<nsIFile> mutexDir;
   nsresult rv = GetSpecialSystemDirectory(OS_TemporaryDirectory,
                                           getter_AddRefs(mutexDir));
   if (NS_SUCCEEDED(rv)) {
     mutexDir->AppendNative(mProgram);
 
     rv = mutexDir->Create(nsIFile::DIRECTORY_TYPE, 0700);
@@ -76,18 +80,19 @@ void nsRemoteService::UnlockStartup() {
 
   if (mRemoteLockDir) {
     mRemoteLockDir->Remove(false);
     mRemoteLockDir = nullptr;
   }
 }
 
 RemoteResult nsRemoteService::StartClient(const char* aDesktopStartupID) {
-  const char* profile;
-  ArgResult ar = CheckArg(gArgc, gArgv, "p", &profile, CheckArgFlag::None);
+  if (mProfile.IsEmpty()) {
+    return REMOTE_NOT_FOUND;
+  }
 
   nsAutoPtr<nsRemoteClient> client;
 
 #ifdef MOZ_WIDGET_GTK
   bool useX11Remote = GDK_IS_X11_DISPLAY(gdk_display_get_default());
 
 #  if defined(MOZ_ENABLE_DBUS)
   if (!useX11Remote) {
@@ -98,17 +103,17 @@ RemoteResult nsRemoteService::StartClien
     client = new nsXRemoteClient();
   }
 
   nsresult rv = client ? client->Init() : NS_ERROR_FAILURE;
   if (NS_FAILED(rv)) return REMOTE_NOT_FOUND;
 
   nsCString response;
   bool success = false;
-  rv = client->SendCommandLine(mProgram.get(), profile, gArgc, gArgv,
+  rv = client->SendCommandLine(mProgram.get(), mProfile.get(), gArgc, gArgv,
                                aDesktopStartupID, getter_Copies(response),
                                &success);
   // did the command fail?
   if (!success) return REMOTE_NOT_FOUND;
 
   // The "command not parseable" error is returned when the
   // nsICommandLineHandler throws a NS_ERROR_ABORT.
   if (response.EqualsLiteral("500 command not parseable"))
@@ -117,35 +122,39 @@ RemoteResult nsRemoteService::StartClien
   if (NS_FAILED(rv)) return REMOTE_NOT_FOUND;
 
   return REMOTE_FOUND;
 #else
   return REMOTE_NOT_FOUND;
 #endif
 }
 
-void nsRemoteService::StartupServer(const char* aProfile) {
+void nsRemoteService::StartupServer() {
   if (mRemoteServer) {
     return;
   }
 
+  if (mProfile.IsEmpty()) {
+    return;
+  }
+
 #ifdef MOZ_WIDGET_GTK
   bool useX11Remote = GDK_IS_X11_DISPLAY(gdk_display_get_default());
 
 #  if defined(MOZ_ENABLE_DBUS)
   if (!useX11Remote) {
     mRemoteServer = MakeUnique<nsDBusRemoteServer>();
   }
 #  endif
   if (useX11Remote) {
     mRemoteServer = MakeUnique<nsGTKRemoteServer>();
   }
 #endif
 
-  nsresult rv = mRemoteServer->Startup(mProgram.get(), aProfile);
+  nsresult rv = mRemoteServer->Startup(mProgram.get(), mProfile.get());
 
   if (NS_FAILED(rv)) {
     mRemoteServer = nullptr;
     return;
   }
 
   nsCOMPtr<nsIObserverService> obs(
       do_GetService("@mozilla.org/observer-service;1"));
--- a/toolkit/components/remote/nsRemoteService.h
+++ b/toolkit/components/remote/nsRemoteService.h
@@ -24,26 +24,28 @@ enum RemoteResult {
 
 class nsRemoteService final : public nsIObserver {
  public:
   // We will be a static singleton, so don't use the ordinary methods.
   NS_DECL_ISUPPORTS
   NS_DECL_NSIOBSERVER
 
   explicit nsRemoteService(const char* aProgram);
+  void SetProfile(nsACString& aProfile);
 
   void LockStartup();
   void UnlockStartup();
 
   RemoteResult StartClient(const char* aDesktopStartupID);
-  void StartupServer(const char* aProfile);
+  void StartupServer();
   void ShutdownServer();
 
  private:
   ~nsRemoteService();
 
   mozilla::UniquePtr<nsRemoteServer> mRemoteServer;
   nsProfileLock mRemoteLock;
   nsCOMPtr<nsIFile> mRemoteLockDir;
   nsCString mProgram;
+  nsCString mProfile;
 };
 
 #endif  // __nsRemoteService_h__
--- a/toolkit/components/remote/nsXRemoteClient.cpp
+++ b/toolkit/components/remote/nsXRemoteClient.cpp
@@ -461,31 +461,32 @@ Window nsXRemoteClient::FindBestWindow(c
 
         XFree(data_return);
       }
     }
 
     // Check to see if there's a profile name on this window.  If
     // there is, then we need to make sure it matches what someone
     // passed in.
-    if (aProfile) {
-      Unused << XGetWindowProperty(
-          mDisplay, w, mMozProfileAtom, 0, (65536 / sizeof(long)), False,
-          XA_STRING, &type, &format, &nitems, &bytesafter, &data_return);
+    Unused << XGetWindowProperty(
+        mDisplay, w, mMozProfileAtom, 0, (65536 / sizeof(long)), False,
+        XA_STRING, &type, &format, &nitems, &bytesafter, &data_return);
 
-      // If there's a profile compare it with what we have
-      if (data_return) {
-        // If the profiles aren't equal, we don't want this window.
-        if (strcmp(aProfile, (const char *)data_return)) {
-          XFree(data_return);
-          continue;
-        }
+    // If there's a profile compare it with what we have
+    if (data_return) {
+      // If the profiles aren't equal, we don't want this window.
+      if (strcmp(aProfile, (const char *)data_return)) {
+        XFree(data_return);
+        continue;
+      }
 
-        XFree(data_return);
-      }
+      XFree(data_return);
+    } else {
+      // This isn't the window for this profile.
+      continue;
     }
 
     // Check to see if the window supports the new command-line passing
     // protocol, if that is requested.
 
     // If we got this far, this is the best window.  It passed
     // all the tests.
     bestWindow = w;
--- a/toolkit/components/remote/nsXRemoteServer.cpp
+++ b/toolkit/components/remote/nsXRemoteServer.cpp
@@ -88,21 +88,19 @@ void nsXRemoteServer::HandleCommandsFor(
                     XA_STRING, 8, PropModeReplace, logname,
                     strlen((char *)logname));
   }
 
   XChangeProperty(mozilla::DefaultXDisplay(), aWindowId, sMozProgramAtom,
                   XA_STRING, 8, PropModeReplace,
                   (unsigned char *)mAppName.get(), mAppName.Length());
 
-  if (!mProfileName.IsEmpty()) {
-    XChangeProperty(mozilla::DefaultXDisplay(), aWindowId, sMozProfileAtom,
-                    XA_STRING, 8, PropModeReplace,
-                    (unsigned char *)mProfileName.get(), mProfileName.Length());
-  }
+  XChangeProperty(mozilla::DefaultXDisplay(), aWindowId, sMozProfileAtom,
+                  XA_STRING, 8, PropModeReplace,
+                  (unsigned char *)mProfileName.get(), mProfileName.Length());
 }
 
 bool nsXRemoteServer::HandleNewProperty(XID aWindowId, Display *aDisplay,
                                         Time aEventTime, Atom aChangedAtom) {
   if (aChangedAtom == sMozCommandLineAtom) {
     // We got a new command atom.
     int result;
     Atom actual_type;
--- a/toolkit/xre/nsAppRunner.cpp
+++ b/toolkit/xre/nsAppRunner.cpp
@@ -3898,30 +3898,16 @@ int XREMain::XRE_mainStartup(bool* aExit
   }
 #endif
 #if defined(MOZ_WIDGET_GTK)
   // handle --remote now that xpcom is fired up
   if (!mDisableRemote) {
     mRemoteService = new nsRemoteService(gAppData->remotingName);
     if (mRemoteService) {
       mRemoteService->LockStartup();
-
-      // Try to remote the entire command line. If this fails, start up
-      // normally.
-      const char* desktopStartupIDPtr =
-          mDesktopStartupID.IsEmpty() ? nullptr : mDesktopStartupID.get();
-
-      RemoteResult rr = mRemoteService->StartClient(desktopStartupIDPtr);
-      if (rr == REMOTE_FOUND) {
-        *aExitFlag = true;
-        return 0;
-      }
-      if (rr == REMOTE_ARG_BAD) {
-        return 1;
-      }
     }
   }
 #endif
 #if defined(MOZ_WIDGET_GTK)
   g_set_application_name(mAppData->name);
   gtk_window_set_auto_startup_notification(false);
 
 #endif /* defined(MOZ_WIDGET_GTK) */
@@ -4035,16 +4021,50 @@ int XREMain::XRE_mainStartup(bool* aExit
   }
 
   if (NS_FAILED(rv)) {
     // We failed to choose or create profile - notify user and quit
     ProfileMissingDialog(mNativeApp);
     return 1;
   }
 
+#if defined(MOZ_WIDGET_GTK)
+  if (mRemoteService) {
+    // We want a unique profile name to identify the remote instance.
+    nsCString profileName;
+    if (profile) {
+      rv = profile->GetName(profileName);
+    }
+    if (!profile || NS_FAILED(rv) || profileName.IsEmpty()) {
+      // Couldn't get a name from the profile. Use the directory name?
+      nsString leafName;
+      rv = mProfD->GetLeafName(leafName);
+      if (NS_SUCCEEDED(rv)) {
+        profileName = NS_ConvertUTF16toUTF8(leafName);
+      }
+    }
+
+    mRemoteService->SetProfile(profileName);
+
+    // Try to remote the entire command line. If this fails, start up
+    // normally.
+    const char* desktopStartupIDPtr =
+        mDesktopStartupID.IsEmpty() ? nullptr : mDesktopStartupID.get();
+
+    RemoteResult rr = mRemoteService->StartClient(desktopStartupIDPtr);
+    if (rr == REMOTE_FOUND) {
+      *aExitFlag = true;
+      return 0;
+    }
+    if (rr == REMOTE_ARG_BAD) {
+      return 1;
+    }
+  }
+#endif
+
   // We always want to lock the profile even if we're actually going to reset
   // it later.
   rv = LockProfile(mNativeApp, mProfD, mProfLD, profile,
                    getter_AddRefs(mProfileLock));
   if (rv == NS_ERROR_LAUNCHED_CHILD_PROCESS || rv == NS_ERROR_ABORT) {
     *aExitFlag = true;
     return 0;
   } else if (NS_FAILED(rv)) {
@@ -4540,17 +4560,17 @@ nsresult XREMain::XRE_mainRun() {
     appStartup->GetShuttingDown(&mShuttingDown);
   }
 
   if (!mShuttingDown) {
 #if defined(MOZ_WIDGET_GTK)
     // if we have X remote support, start listening for requests on the
     // proxy window.
     if (mRemoteService) {
-      mRemoteService->StartupServer(mProfileName.get());
+      mRemoteService->StartupServer();
       mRemoteService->UnlockStartup();
     }
 #endif /* MOZ_WIDGET_GTK */
 
     mNativeApp->Enable();
   }
 
 #ifdef MOZ_INSTRUMENT_EVENT_LOOP