From b2d9af5e155dd8df043ec3ba1016b4ab7cce1fd0 Mon Sep 17 00:00:00 2001 From: Andrey Lushnikov Date: Fri, 19 Feb 2021 10:32:47 -0800 Subject: [PATCH] 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 --- .../checkout_build_archive_upload.sh | 3 +- browser_patches/firefox/BUILD_NUMBER | 4 +- browser_patches/firefox/build.sh | 19 ++++-- .../firefox/patches/bootstrap.diff | 61 ++++++++++++++----- 4 files changed, 63 insertions(+), 24 deletions(-) diff --git a/browser_patches/checkout_build_archive_upload.sh b/browser_patches/checkout_build_archive_upload.sh index c126a3a41f..71f9c5d95c 100755 --- 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 index 6ca12d2513..62133f6fb2 100644 --- 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 index 620bb91de4..91d86c0186 100755 --- 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,7 +73,6 @@ 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; @@ -76,8 +80,9 @@ 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" @@ -85,6 +90,10 @@ if [[ $1 == "--full" ]]; then 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 index 2a6b127ab6..118d7b2559 100644 --- 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 #include -@@ -324,8 +325,25 @@ Maybe LauncherMain(int& argc, wchar_t* argv[], +@@ -324,8 +325,19 @@ Maybe 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(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(stdio3), + reinterpret_cast(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 = 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 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 + #include + + #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(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