From d2536d7cf5fa731250bbfd0d18959cacc8afffac Mon Sep 17 00:00:00 2001
From: Evan Carroll <me@evancarroll.com>
Date: Tue, 20 Dec 2022 07:50:02 -0600
Subject: [PATCH] feat: Default to not downloading if explicit browser path is
 set (#9440)

Fixed: #9419
---
 docs/troubleshooting.md                    |  3 +-
 packages/puppeteer/src/getConfiguration.ts | 83 +++++++++++++---------
 2 files changed, 49 insertions(+), 37 deletions(-)

diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md
index 2036da56059..38cd1313f54 100644
--- a/docs/troubleshooting.md
+++ b/docs/troubleshooting.md
@@ -433,8 +433,7 @@ RUN apk add --no-cache \
 ...
 
 # Tell Puppeteer to skip installing Chrome. We'll be using the installed package.
-ENV PUPPETEER_SKIP_CHROMIUM_DOWNLOAD=true \
-    PUPPETEER_EXECUTABLE_PATH=/usr/bin/chromium-browser
+ENV PUPPETEER_EXECUTABLE_PATH=/usr/bin/chromium-browser
 
 # Puppeteer v13.5.0 works with Chromium 100.
 RUN yarn add puppeteer@13.5.0
diff --git a/packages/puppeteer/src/getConfiguration.ts b/packages/puppeteer/src/getConfiguration.ts
index 4eb8acaa102..8bbeb777130 100644
--- a/packages/puppeteer/src/getConfiguration.ts
+++ b/packages/puppeteer/src/getConfiguration.ts
@@ -24,38 +24,60 @@ export const getConfiguration = (): Configuration => {
   const configuration: Configuration = result ? result.config : {};
 
   // Merging environment variables.
-  configuration.browserRevision =
-    process.env['PUPPETEER_CHROMIUM_REVISION'] ??
-    process.env['PUPPETEER_BROWSER_REVISION'] ??
-    process.env['npm_config_puppeteer_browser_revision'] ??
-    process.env['npm_package_config_puppeteer_browser_revision'] ??
-    configuration.browserRevision;
+  configuration.defaultProduct = (process.env['PUPPETEER_PRODUCT'] ??
+    process.env['npm_config_puppeteer_product'] ??
+    process.env['npm_package_config_puppeteer_product'] ??
+    configuration.defaultProduct ??
+    'chrome') as Product;
+
+  configuration.executablePath =
+    process.env['PUPPETEER_EXECUTABLE_PATH'] ??
+    process.env['npm_config_puppeteer_executable_path'] ??
+    process.env['npm_package_config_puppeteer_executable_path'] ??
+    configuration.executablePath;
+
+  // Default to skipDownload if executablePath is set
+  if (configuration.executablePath) {
+    configuration.skipDownload = true;
+  }
+
+  // Set skipDownload explicitly or from default
+  configuration.skipDownload = Boolean(
+    process.env['PUPPETEER_SKIP_DOWNLOAD'] ??
+      process.env['npm_config_puppeteer_skip_download'] ??
+      process.env['npm_package_config_puppeteer_skip_download'] ??
+      process.env['PUPPETEER_SKIP_CHROMIUM_DOWNLOAD'] ??
+      process.env['npm_config_puppeteer_skip_chromium_download'] ??
+      process.env['npm_package_config_puppeteer_skip_chromium_download'] ??
+      configuration.skipDownload
+  );
+
+  // Prepare variables used in browser downloading
+  if (!configuration.skipDownload) {
+    configuration.browserRevision =
+      process.env['PUPPETEER_CHROMIUM_REVISION'] ??
+      process.env['PUPPETEER_BROWSER_REVISION'] ??
+      process.env['npm_config_puppeteer_browser_revision'] ??
+      process.env['npm_package_config_puppeteer_browser_revision'] ??
+      configuration.browserRevision;
+    configuration.downloadHost =
+      process.env['PUPPETEER_DOWNLOAD_HOST'] ??
+      process.env['npm_config_puppeteer_download_host'] ??
+      process.env['npm_package_config_puppeteer_download_host'] ??
+      configuration.downloadHost;
+    configuration.downloadPath =
+      process.env['PUPPETEER_DOWNLOAD_PATH'] ??
+      process.env['npm_config_puppeteer_download_path'] ??
+      process.env['npm_package_config_puppeteer_download_path'] ??
+      configuration.downloadPath;
+  }
+
   configuration.cacheDirectory =
     process.env['PUPPETEER_CACHE_DIR'] ??
     process.env['npm_config_puppeteer_cache_dir'] ??
     process.env['npm_package_config_puppeteer_cache_dir'] ??
     configuration.cacheDirectory ??
     join(homedir(), '.cache', 'puppeteer');
-  configuration.downloadHost =
-    process.env['PUPPETEER_DOWNLOAD_HOST'] ??
-    process.env['npm_config_puppeteer_download_host'] ??
-    process.env['npm_package_config_puppeteer_download_host'] ??
-    configuration.downloadHost;
-  configuration.downloadPath =
-    process.env['PUPPETEER_DOWNLOAD_PATH'] ??
-    process.env['npm_config_puppeteer_download_path'] ??
-    process.env['npm_package_config_puppeteer_download_path'] ??
-    configuration.downloadPath;
-  configuration.executablePath =
-    process.env['PUPPETEER_EXECUTABLE_PATH'] ??
-    process.env['npm_config_puppeteer_executable_path'] ??
-    process.env['npm_package_config_puppeteer_executable_path'] ??
-    configuration.executablePath;
-  configuration.defaultProduct = (process.env['PUPPETEER_PRODUCT'] ??
-    process.env['npm_config_puppeteer_product'] ??
-    process.env['npm_package_config_puppeteer_product'] ??
-    configuration.defaultProduct ??
-    'chrome') as Product;
   configuration.temporaryDirectory =
     process.env['PUPPETEER_TMP_DIR'] ??
     process.env['npm_config_puppeteer_tmp_dir'] ??
@@ -72,15 +94,6 @@ export const getConfiguration = (): Configuration => {
       configuration.experiments.macArmChromiumEnabled
   );
 
-  configuration.skipDownload = Boolean(
-    process.env['PUPPETEER_SKIP_DOWNLOAD'] ??
-      process.env['npm_config_puppeteer_skip_download'] ??
-      process.env['npm_package_config_puppeteer_skip_download'] ??
-      process.env['PUPPETEER_SKIP_CHROMIUM_DOWNLOAD'] ??
-      process.env['npm_config_puppeteer_skip_chromium_download'] ??
-      process.env['npm_package_config_puppeteer_skip_chromium_download'] ??
-      configuration.skipDownload
-  );
   configuration.logLevel = (process.env['PUPPETEER_LOGLEVEL'] ??
     process.env['npm_config_LOGLEVEL'] ??
     process.env['npm_package_config_LOGLEVEL'] ??