Bug 1654100 - Measure the elapsed time of ShellExecuteByExplorer. r=froydnj
authorToshihito Kikuchi <tkikuchi@mozilla.com>
Thu, 23 Jul 2020 19:57:35 +0000
changeset 541826 6e556d9c084532ca27fcf30353a88eb82b775595
parent 541825 2879ab33ffccb5e681f615877a09084febf3caf6
child 541827 cc83430bf63ad08d3d955d0c43ec9461a9988453
push id37633
push userccoroiu@mozilla.com
push dateFri, 24 Jul 2020 09:32:06 +0000
treeherdermozilla-central@141543043270 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1654100, 1646986
milestone80.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 1654100 - Measure the elapsed time of ShellExecuteByExplorer. r=froydnj This patch adds a new measurement to the histogram about the elapsed time of `ShellExecuteByExplorer`. If ShellExecuteByExplorer takes long in a considerable number of instances, we need to consider fixing bug 1646986. Differential Revision: https://phabricator.services.mozilla.com/D84407
toolkit/components/telemetry/Histograms.json
uriloader/exthandler/win/nsMIMEInfoWin.cpp
xpcom/io/nsLocalFileWin.cpp
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -11266,16 +11266,28 @@
       "JAVASCRIPT",
       "IMAGE",
       "MEDIA",
       "STYLESHEET",
       "WASM"
     ],
     "description": "Percentage of the entries with the given content type. Numbers are sampled periodically, every time 2GB of data is written to the cache."
   },
+  "SHELLEXECUTEBYEXPLORER_DURATION_MS" : {
+    "record_in_processes": ["main"],
+    "products": ["firefox"],
+    "alert_emails": ["tkikuchi@mozilla.com"],
+    "bug_numbers": [1654100],
+    "expires_in_version": "never",
+    "kind": "exponential",
+    "high": 100000,
+    "n_buckets": 20,
+    "description": "Time spent executing a downloaded file via explorer.exe (ms)",
+    "operating_systems": ["windows"]
+  },
   "SSL_TLS13_INTOLERANCE_REASON_PRE": {
     "record_in_processes": ["main", "content"],
     "products": ["firefox", "fennec"],
     "alert_emails": ["seceng-telemetry@mozilla.com"],
     "bug_numbers": [1250568],
     "expires_in_version": "never",
     "kind": "enumerated",
     "n_values": 64,
--- a/uriloader/exthandler/win/nsMIMEInfoWin.cpp
+++ b/uriloader/exthandler/win/nsMIMEInfoWin.cpp
@@ -17,16 +17,17 @@
 #include "windows.h"
 #include "nsIWindowsRegKey.h"
 #include "nsUnicharUtils.h"
 #include "nsITextToSubURI.h"
 #include "nsVariant.h"
 #include "mozilla/CmdLineAndEnvUtils.h"
 #include "mozilla/ShellHeaderOnlyUtils.h"
 #include "mozilla/StaticPrefs_browser.h"
+#include "mozilla/Telemetry.h"
 #include "mozilla/UrlmonHeaderOnlyUtils.h"
 #include "mozilla/UniquePtrExtensions.h"
 
 #define RUNDLL32_EXE L"\\rundll32.exe"
 
 NS_IMPL_ISUPPORTS_INHERITED(nsMIMEInfoWin, nsMIMEInfoBase, nsIPropertyBag)
 
 nsMIMEInfoWin::~nsMIMEInfoWin() {}
@@ -68,26 +69,30 @@ nsresult nsMIMEInfoWin::ShellExecuteWith
   _variant_t showCmd(SW_SHOWNORMAL);
 
   // Ask Explorer to ShellExecute on our behalf, as some applications such as
   // Skype for Business do not start correctly when inheriting our process's
   // migitation policies.
   // It does not work in a special environment such as Citrix.  In such a case
   // we fall back to launching an application as a child process.  We need to
   // find a way to handle the combination of these interop issues.
-  mozilla::LauncherVoidResult shellExecuteOk = mozilla::ShellExecuteByExplorer(
-      execPathBStr, assembledArgs.get(), verbDefault, workingDir, showCmd);
-  if (shellExecuteOk.isErr()) {
-    // No need to pass assembledArgs to LaunchWithIProcess.  aArgv will be
-    // processed in nsProcess::RunProcess.
-    return LaunchWithIProcess(aExecutable, aArgc,
-                              reinterpret_cast<const char16_t**>(aArgv));
+  {
+    Telemetry::AutoTimer<Telemetry::SHELLEXECUTEBYEXPLORER_DURATION_MS> timer;
+    mozilla::LauncherVoidResult shellExecuteOk =
+        mozilla::ShellExecuteByExplorer(execPathBStr, assembledArgs.get(),
+                                        verbDefault, workingDir, showCmd);
+    if (shellExecuteOk.isOk()) {
+      return NS_OK;
+    }
   }
 
-  return NS_OK;
+  // No need to pass assembledArgs to LaunchWithIProcess.  aArgv will be
+  // processed in nsProcess::RunProcess.
+  return LaunchWithIProcess(aExecutable, aArgc,
+                            reinterpret_cast<const char16_t**>(aArgv));
 }
 
 NS_IMETHODIMP
 nsMIMEInfoWin::LaunchWithFile(nsIFile* aFile) {
   nsresult rv;
 
   // it doesn't make any sense to call this on protocol handlers
   NS_ASSERTION(mClass == eMIMEInfo,
@@ -317,21 +322,24 @@ nsresult nsMIMEInfoWin::LoadUriInternal(
     // if a uri to open includes credentials.  This does not happen in Firefox
     // because Firefox does not have to launch a process to open a uri.
     //
     // Since Thunderbird does not use mitigation policies which could cause
     // compatibility issues, we get no benefit from using
     // ShellExecuteByExplorer.  Thus we skip it and go straight to
     // ShellExecuteExW for Thunderbird.
 #ifndef MOZ_THUNDERBIRD
-    mozilla::LauncherVoidResult shellExecuteOk =
-        mozilla::ShellExecuteByExplorer(validatedUri.inspect(), args, verb,
-                                        workingDir, showCmd);
-    if (shellExecuteOk.isOk()) {
-      return NS_OK;
+    {
+      Telemetry::AutoTimer<Telemetry::SHELLEXECUTEBYEXPLORER_DURATION_MS> timer;
+      mozilla::LauncherVoidResult shellExecuteOk =
+          mozilla::ShellExecuteByExplorer(validatedUri.inspect(), args, verb,
+                                          workingDir, showCmd);
+      if (shellExecuteOk.isOk()) {
+        return NS_OK;
+      }
     }
 #endif  // MOZ_THUNDERBIRD
 
     SHELLEXECUTEINFOW sinfo = {sizeof(sinfo)};
     sinfo.fMask = SEE_MASK_NOASYNC;
     sinfo.lpVerb = V_BSTR(&verb);
     sinfo.nShow = showCmd;
     sinfo.lpFile = validatedUri.inspect();
--- a/xpcom/io/nsLocalFileWin.cpp
+++ b/xpcom/io/nsLocalFileWin.cpp
@@ -51,16 +51,17 @@
 #include "nsThreadUtils.h"
 #include "nsXULAppAPI.h"
 #include "nsIWindowMediator.h"
 
 #include "mozIDOMWindow.h"
 #include "nsPIDOMWindow.h"
 #include "nsIWidget.h"
 #include "mozilla/ShellHeaderOnlyUtils.h"
+#include "mozilla/Telemetry.h"
 #include "mozilla/WidgetUtils.h"
 #include "WinUtils.h"
 
 using namespace mozilla;
 using mozilla::FilePreferences::kDevicePathSpecifier;
 using mozilla::FilePreferences::kPathSeparator;
 
 #define CHECK_mWorkingPath()                                     \
@@ -3041,16 +3042,17 @@ nsLocalFile::Launch() {
   if (NS_FAILED(rv)) {
     isExecutable = false;
   }
 
   // If the file is an executable, go straight to ShellExecuteExW.
   // Otherwise try ShellExecuteByExplorer first, and if it fails,
   // run ShellExecuteExW.
   if (!isExecutable) {
+    Telemetry::AutoTimer<Telemetry::SHELLEXECUTEBYEXPLORER_DURATION_MS> timer;
     mozilla::LauncherVoidResult shellExecuteOk =
         mozilla::ShellExecuteByExplorer(execPath, args, verbDefault,
                                         workingDirectoryPtr, showCmd);
     if (shellExecuteOk.isOk()) {
       return NS_OK;
     }
   }