browser(firefox): properly initialize debugging pipe on windows (#5514)

browser(firefox): properly initialize debugging pipe on windows Firefox on Windows has 2 launch modes: - default: a special "launcher process" is used to start browser as a sub-process - non-default: browser process starts right away Firefox has a logic to detect how successful was the use of the launcher process to do self-recovery when things go wrong. Namely: - when attempting to use launcher process, firefox records a timestamp of the attempt beginning - once the launcher process successfully launches browser sub-process, firefox records another timestamp of the completion On a new launch, firefox checks what timestamps are present. If there's a timestamp that signifies start of launcher process, but no successful timestamp, it decides that last "launcher process" use was not successful and falls back to launching browser right away. When launching 2 firefox processes right away, the first process uses attempts to use launcher process and records the first timestamp. At the same time, the second instance sees the first timestamp and doesn't see the second timestamp, and falls back to launching browser right away. Our debugging pipe code, however, does not support non-launcher-process code path. This patch adds support for remote debugging pipe in case of non-launcher-process startup. Drive-by: - disable crashreporter altogether - remove stray dcheck that breaks firefox debug compilation - disable compilation of firefox update agent - do not use WIN32_DISTRIB flag unless doing full builds since it kills incremental compilation References #4660
microsoft · Feb 19, 2021 · b2d9af5 · b2d9af5
1 parent 48f7a37
commit b2d9af5
Show file tree

Hide file tree

Showing 4 changed files with 63 additions and 24 deletions.
diff --git a/browser_patches/checkout_build_archive_upload.sh b/browser_patches/checkout_build_archive_upload.sh
@@ -172,11 +172,12 @@ elif [[ "$BUILD_FLAVOR" == "firefox-mac-11.0-arm64" ]]; then
   BUILD_BLOB_NAME="firefox-mac-11.0-arm64.zip"
 elif [[ "$BUILD_FLAVOR" == "firefox-win32" ]]; then
   BROWSER_NAME="firefox"
+  EXTRA_BUILD_ARGS="--full"
   EXPECTED_HOST_OS="MINGW"
   BUILD_BLOB_NAME="firefox-win32.zip"
 elif [[ "$BUILD_FLAVOR" == "firefox-win64" ]]; then
   BROWSER_NAME="firefox"
-  EXTRA_BUILD_ARGS="--win64"
+  EXTRA_BUILD_ARGS="--win64 --full"
   EXPECTED_HOST_OS="MINGW"
   BUILD_BLOB_NAME="firefox-win64.zip"
 

diff --git a/browser_patches/firefox/BUILD_NUMBER b/browser_patches/firefox/BUILD_NUMBER
@@ -1,2 +1,2 @@
-1230
-Changed: dgozman@gmail.com Sat Feb 13 10:15:07 PST 2021
+1231
+Changed: lushnikov@chromium.org Fri Feb 19 10:28:40 MST 2021
diff --git a/browser_patches/firefox/build.sh b/browser_patches/firefox/build.sh
@@ -22,6 +22,8 @@ else
 fi
 
 
+rm .mozconfig
+
 if [[ "$(uname)" == "Darwin" ]]; then
   if [[ $(uname -m) == "arm64" ]]; then
     # Building on Apple Silicon requires XCode12.2 and does not require any extra SDKs.
@@ -45,18 +47,21 @@ if [[ "$(uname)" == "Darwin" ]]; then
       exit 1
     else
       echo "-- configuting .mozconfig with ${MACOS_SDK_VERSION} SDK path"
-      echo "ac_add_options --with-macos-sdk=$HOME/SDK-archive/MacOSX${MACOS_SDK_VERSION}.sdk/" > .mozconfig
+      echo "ac_add_options --with-macos-sdk=$HOME/SDK-archive/MacOSX${MACOS_SDK_VERSION}.sdk/" >> .mozconfig
     fi
   fi
   echo "-- building on Mac"
 elif [[ "$(uname)" == "Linux" ]]; then
   echo "-- building on Linux"
-  echo "ac_add_options --disable-av1" > .mozconfig
+  echo "ac_add_options --disable-av1" >> .mozconfig
 elif [[ "$(uname)" == MINGW* ]]; then
+  echo "ac_add_options --disable-update-agent" >> .mozconfig
+  echo "ac_add_options --disable-default-browser-agent" >> .mozconfig
+
   DLL_FILE=""
   if [[ $1 == "--win64" ]]; then
     echo "-- building win64 build on MINGW"
-    echo "ac_add_options --target=x86_64-pc-mingw32" > .mozconfig
+    echo "ac_add_options --target=x86_64-pc-mingw32" >> .mozconfig
     echo "ac_add_options --host=x86_64-pc-mingw32" >> .mozconfig
     DLL_FILE=$("C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe" -latest -find '**\Redist\MSVC\*\x64\**\vcruntime140.dll')
   else
@@ -68,23 +73,27 @@ elif [[ "$(uname)" == MINGW* ]]; then
     echo "ERROR: cannot find MS VS C++ redistributable $WIN32_REDIST_DIR"
     exit 1;
   fi
-  echo "export WIN32_REDIST_DIR=\"$WIN32_REDIST_DIR\"" >> .mozconfig
 else
   echo "ERROR: cannot upload on this platform!" 1>&2
   exit 1;
 fi
 
 OBJ_FOLDER="obj-build-playwright"
 echo "mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/${OBJ_FOLDER}" >> .mozconfig
+echo "ac_add_options --disable-crashreporter" >> .mozconfig
 
-if [[ $1 == "--full" ]]; then
+if [[ $1 == "--full" || $2 == "--full" ]]; then
   if [[ "$(uname)" == "Darwin" && "$(uname -m)" == "arm64" ]]; then
     ./mach artifact toolchain --from-build macosx64-node
     rm -rf "$HOME/.mozbuild/node"
     mv node "$HOME/.mozbuild/"
   elif [[ "$(uname)" == "Darwin" || "$(uname)" == "Linux" ]]; then
     SHELL=/bin/sh ./mach bootstrap --application-choice=browser --no-interactive --no-system-changes
   fi
+  if [[ ! -z "${WIN32_REDIST_DIR}" ]]; then
+    # Having this option in .mozconfig kills incremental compilation.
+    echo "export WIN32_REDIST_DIR=\"$WIN32_REDIST_DIR\"" >> .mozconfig
+  fi
 fi
 
 if ! [[ -f "$HOME/.mozbuild/_virtualenvs/mach/bin/python" ]]; then

diff --git a/browser_patches/firefox/patches/bootstrap.diff b/browser_patches/firefox/patches/bootstrap.diff
@@ -59,7 +59,7 @@ index f042cc1081850ac60e329b70b5569f8b97d4e4dc..65bcff9b41b9471ef1427e3ea330481c
     * Return XPCOM wrapper for the internal accessible.
     */
 diff --git a/browser/app/winlauncher/LauncherProcessWin.cpp b/browser/app/winlauncher/LauncherProcessWin.cpp
-index faf53eff9a46f958890d2c50de4c741fce75f1b1..7ad4b9f2203dbf1706518a0ba372d424673c9d64 100644
+index faf53eff9a46f958890d2c50de4c741fce75f1b1..63a102074664f972bfb16ba7f57af730abad9eea 100644
 --- a/browser/app/winlauncher/LauncherProcessWin.cpp
 +++ b/browser/app/winlauncher/LauncherProcessWin.cpp
 @@ -23,6 +23,7 @@
@@ -70,7 +70,7 @@ index faf53eff9a46f958890d2c50de4c741fce75f1b1..7ad4b9f2203dbf1706518a0ba372d424
  #include <windows.h>
  #include <processthreadsapi.h>
 
-@@ -324,8 +325,25 @@ Maybe<int> LauncherMain(int& argc, wchar_t* argv[],
+@@ -324,8 +325,19 @@ Maybe<int> LauncherMain(int& argc, wchar_t* argv[],
    HANDLE stdHandles[] = {::GetStdHandle(STD_INPUT_HANDLE),
                           ::GetStdHandle(STD_OUTPUT_HANDLE),
                           ::GetStdHandle(STD_ERROR_HANDLE)};
@@ -81,15 +81,9 @@ index faf53eff9a46f958890d2c50de4c741fce75f1b1..7ad4b9f2203dbf1706518a0ba372d424
 +      mozilla::CheckArg(argc, argv, L"juggler-pipe",
 +                        static_cast<const wchar_t**>(nullptr),
 +                        mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND;
-+  if (hasJugglerPipe && !mozilla::EnvHasValue("PW_PIPE_READ")) {
++  if (hasJugglerPipe) {
 +    intptr_t stdio3 = _get_osfhandle(3);
 +    intptr_t stdio4 = _get_osfhandle(4);
-+    CHAR stdio3str[20];
-+    CHAR stdio4str[20];
-+    itoa(stdio3, stdio3str, 10);
-+    itoa(stdio4, stdio4str, 10);
-+    SetEnvironmentVariable("PW_PIPE_READ", stdio3str);
-+    SetEnvironmentVariable("PW_PIPE_WRITE", stdio4str);
 +    HANDLE pipeHandles[] = {reinterpret_cast<HANDLE>(stdio3),
 +                            reinterpret_cast<HANDLE>(stdio4)};
 +    attrs.AddInheritableHandles(pipeHandles);
@@ -178,7 +172,7 @@ index 040c7b124dec6bb254563bbe74fe50012cb077a3..b4e6b8132786af70e8ad0dce88b67c28
    const transportProvider = {
      setListener(upgradeListener) {
 diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
-index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec2c117442 100644
+index ee271e6d7fd265aea278d841f66340a9c70e59f9..0b3cbdd0a2d1c78c2f1356994a72d2bacb45afc6 100644
 --- a/docshell/base/nsDocShell.cpp
 +++ b/docshell/base/nsDocShell.cpp
 @@ -15,6 +15,12 @@
@@ -247,7 +241,7 @@ index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec
    if (!isSubFrame && !isRoot) {
      /*
       * We don't want to send OnLocationChange notifications when
-@@ -3256,6 +3273,204 @@ nsDocShell::GetMessageManager(ContentFrameMessageManager** aMessageManager) {
+@@ -3256,6 +3273,203 @@ nsDocShell::GetMessageManager(ContentFrameMessageManager** aMessageManager) {
    return NS_OK;
  }
 
@@ -310,7 +304,6 @@ index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec
 +
 +NS_IMETHODIMP
 +nsDocShell::GetLanguageOverride(nsAString& aLanguageOverride) {
-+  MOZ_ASSERT(aEnabled);
 +  aLanguageOverride = GetRootDocShell()->mLanguageOverride;
 +  return NS_OK;
 +}
@@ -452,7 +445,7 @@ index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec
  NS_IMETHODIMP
  nsDocShell::GetIsNavigating(bool* aOut) {
    *aOut = mIsNavigating;
-@@ -4864,7 +5079,7 @@ nsDocShell::GetIsOffScreenBrowser(bool* aIsOffScreen) {
+@@ -4864,7 +5078,7 @@ nsDocShell::GetIsOffScreenBrowser(bool* aIsOffScreen) {
  }
 
  void nsDocShell::ActivenessMaybeChanged() {
@@ -461,7 +454,7 @@ index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec
    if (RefPtr<PresShell> presShell = GetPresShell()) {
      presShell->SetIsActive(isActive);
    }
-@@ -8589,6 +8804,12 @@ nsresult nsDocShell::PerformRetargeting(nsDocShellLoadState* aLoadState) {
+@@ -8589,6 +8803,12 @@ nsresult nsDocShell::PerformRetargeting(nsDocShellLoadState* aLoadState) {
                       true,  // aForceNoOpener
                       getter_AddRefs(newBC));
        MOZ_ASSERT(!newBC);
@@ -474,7 +467,7 @@ index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec
        return rv;
      }
 
-@@ -12548,6 +12769,9 @@ class OnLinkClickEvent : public Runnable {
+@@ -12548,6 +12768,9 @@ class OnLinkClickEvent : public Runnable {
        mHandler->OnLinkClickSync(mContent, mLoadState, mNoOpenerImplied,
                                  mTriggeringPrincipal);
      }
@@ -484,7 +477,7 @@ index ee271e6d7fd265aea278d841f66340a9c70e59f9..80665230adf04130e5bf79a3a2b09bec
      return NS_OK;
    }
 
-@@ -12633,6 +12857,8 @@ nsresult nsDocShell::OnLinkClick(
+@@ -12633,6 +12856,8 @@ nsresult nsDocShell::OnLinkClick(
    nsCOMPtr<nsIRunnable> ev =
        new OnLinkClickEvent(this, aContent, loadState, noOpenerImplied,
                             aIsTrusted, aTriggeringPrincipal);
@@ -1868,6 +1861,42 @@ index bbc3c98e4885f23f03f83b7c2aa00e4eb4faaef5..45dcb084904c1d2729ef1e2cd1bef1a4
          '/toolkit/components/telemetry/tests/marionette',
      ]
 
+diff --git a/toolkit/xre/nsWindowsWMain.cpp b/toolkit/xre/nsWindowsWMain.cpp
+index 109c53cac98302d657d2a5a997f2ba687db14515..4d3c4beddaf627441e28f2a49d793d56fe4e2447 100644
+--- a/toolkit/xre/nsWindowsWMain.cpp
++++ b/toolkit/xre/nsWindowsWMain.cpp
+@@ -14,8 +14,10 @@
+ #endif
+
+ #include "mozilla/Char16.h"
++#include "mozilla/CmdLineAndEnvUtils.h"
+ #include "nsUTF8Utils.h"
+
++#include <io.h>
+ #include <windows.h>
+
+ #ifdef __MINGW32__
+@@ -94,6 +96,20 @@ static void FreeAllocStrings(int argc, char** argv) {
+ int wmain(int argc, WCHAR** argv) {
+   SanitizeEnvironmentVariables();
+   SetDllDirectoryW(L"");
++  bool hasJugglerPipe =
++      mozilla::CheckArg(argc, argv, L"juggler-pipe",
++                        static_cast<const wchar_t**>(nullptr),
++                        mozilla::CheckArgFlag::None) == mozilla::ARG_FOUND;
++  if (hasJugglerPipe && !mozilla::EnvHasValue("PW_PIPE_READ")) {
++    intptr_t stdio3 = _get_osfhandle(3);
++    intptr_t stdio4 = _get_osfhandle(4);
++    CHAR stdio3str[20];
++    CHAR stdio4str[20];
++    itoa(stdio3, stdio3str, 10);
++    itoa(stdio4, stdio4str, 10);
++    SetEnvironmentVariableA("PW_PIPE_READ", stdio3str);
++    SetEnvironmentVariableA("PW_PIPE_WRITE", stdio4str);
++  }
+
+   // Only run this code if LauncherProcessWin.h was included beforehand, thus
+   // signalling that the hosting process should support launcher mode.
 diff --git a/uriloader/base/nsDocLoader.cpp b/uriloader/base/nsDocLoader.cpp
 index bf7902f5dd642e2800b3fa83f4d379ab1ac9fda0..b94c24f9534cc19a5c2828e035e33c95f671d181 100644
 --- a/uriloader/base/nsDocLoader.cpp