Backed out changeset 2a43fb1b7fef (bug 1750253) for causing bc failures in browser_local_files_open_doesnt_duplicate.js

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -63,7 +63,6 @@
 
 #include "nsDSURIContentListener.h"
 #include "nsMimeTypes.h"
-#include "nsMIMEInfoImpl.h"
 // used for header disposition information.
 #include "nsIHttpChannel.h"
 #include "nsIHttpChannelInternal.h"
@@ -77,6 +76,7 @@
 #  include "nsILocalFileMac.h"
 #endif
 
+#include "nsPluginHost.h"
 #include "nsEscape.h"
 
 #include "nsIStringBundle.h"  // XXX needed to localize error msgs
@@ -1955,33 +1955,6 @@ NS_IMETHODIMP nsExternalAppHandler::OnStartRequest(nsIRequest* request) {
                 !shouldAutomaticallyHandleInternally;
   }
 
-  // If we're handling with the OS default and we are that default, force
-  // asking, so we don't end up in an infinite loop:
-  if (!alwaysAsk && action == nsIMIMEInfo::useSystemDefault) {
-    bool areOSDefault = false;
-    alwaysAsk = NS_SUCCEEDED(mMimeInfo->IsCurrentAppOSDefault(&areOSDefault)) &&
-                areOSDefault;
-  } else if (!alwaysAsk && action == nsIMIMEInfo::useHelperApp) {
-    nsCOMPtr<nsIHandlerApp> preferredApp;
-    mMimeInfo->GetPreferredApplicationHandler(getter_AddRefs(preferredApp));
-    nsCOMPtr<nsILocalHandlerApp> handlerApp = do_QueryInterface(preferredApp);
-    if (handlerApp) {
-      nsCOMPtr<nsIFile> executable;
-      handlerApp->GetExecutable(getter_AddRefs(executable));
-      nsCOMPtr<nsIFile> ourselves;
-      if (executable &&
-          // Despite the name, this really just fetches an nsIFile...
-          NS_SUCCEEDED(NS_GetSpecialDirectory(XRE_EXECUTABLE_FILE,
-                                              getter_AddRefs(ourselves)))) {
-        ourselves = nsMIMEInfoBase::GetCanonicalExecutable(ourselves);
-        executable = nsMIMEInfoBase::GetCanonicalExecutable(executable);
-        bool isSameApp = false;
-        alwaysAsk =
-            NS_FAILED(executable->Equals(ourselves, &isSameApp)) || isSameApp;
-      }
-    }
-  }
-
   // if we were told that we _must_ save to disk without asking, all the stuff
   // before this is irrelevant; override it
   if (mForceSave) {

uriloader/exthandler/nsMIMEInfoImpl.cpp

@@ -21,20 +21,9 @@
 static bool sInitializedOurData = false;
 StaticRefPtr<nsIFile> sOurAppFile;
 
-/* static */
-already_AddRefed<nsIFile> nsMIMEInfoBase::GetCanonicalExecutable(
-    nsIFile* aFile) {
+static already_AddRefed<nsIFile> GetCanonicalExecutable(nsIFile* aFile) {
   nsCOMPtr<nsIFile> binary = aFile;
 #ifdef XP_MACOSX
-  nsAutoString path;
-  if (binary) {
-    binary->GetPath(path);
-  }
-  if (!StringEndsWith(path, u".app"_ns) && path.RFind(u".app/"_ns) == -1) {
-    // This shouldn't ever happen with Firefox's own binary, tracked in
-    // sOurAppFile, but might happen when called with other files.
-    return binary.forget();
-  }
   nsAutoString leafName;
   if (binary) {
     binary->GetLeafName(leafName);
@@ -42,7 +31,7 @@ already_AddRefed<nsIFile> nsMIMEInfoBase::GetCanonicalExecutable(
   while (binary && !StringEndsWith(leafName, u".app"_ns)) {
     nsCOMPtr<nsIFile> parent;
     binary->GetParent(getter_AddRefs(parent));
-    binary = std::move(parent);
+    binary = parent;
     if (binary) {
       binary->GetLeafName(leafName);
     }
@@ -58,7 +47,7 @@ static void EnsureAppDetailsAvailable() {
   sInitializedOurData = true;
   nsCOMPtr<nsIFile> binary;
   XRE_GetBinaryPath(getter_AddRefs(binary));
-  sOurAppFile = nsMIMEInfoBase::GetCanonicalExecutable(binary);
+  sOurAppFile = GetCanonicalExecutable(binary);
   ClearOnShutdown(&sOurAppFile);
 }
 

uriloader/exthandler/nsMIMEInfoImpl.h

@@ -95,8 +95,6 @@ class nsMIMEInfoBase : public nsIMIMEInfo {
    */
   bool HasExtensions() const { return mExtensions.Length() != 0; }
 
-  static already_AddRefed<nsIFile> GetCanonicalExecutable(nsIFile* aFile);
-
  protected:
   virtual ~nsMIMEInfoBase();  // must be virtual, as the the base class's
                               // Release should call the subclass's destructor

uriloader/exthandler/tests/mochitest/browser.ini

@@ -54,7 +54,6 @@ support-files =
 support-files =
   file_as.exe
   file_as.exe^headers^
-[browser_filehandling_loop.js]
 [browser_launched_app_save_directory.js]
  # This test checks the save destination of the
  # open with app download on Windows, Linux and OS X.

uriloader/exthandler/tests/mochitest/browser_filehandling_loop.js

@@ -1,93 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-/**
- * If the user has set Firefox itself as a helper app,
- * we should force prompting what to do, rather than ending up
- * in an infinite loop.
- * In an ideal world, we'd also test the case where we are the OS
- * default handler app, but that would require test infrastructure
- * to make ourselves the OS default (or at least fool ourselves into
- * believing we are) which we don't have...
- */
-add_task(async function test_helperapp() {
-  // Set up the test infrastructure:
-  const mimeSvc = Cc["@mozilla.org/mime;1"].getService(Ci.nsIMIMEService);
-  const handlerSvc = Cc["@mozilla.org/uriloader/handler-service;1"].getService(
-    Ci.nsIHandlerService
-  );
-  let handlerInfo = mimeSvc.getFromTypeAndExtension("application/x-foo", "foo");
-  registerCleanupFunction(() => {
-    handlerSvc.remove(handlerInfo);
-  });
-  // Say we want to use a specific app:
-  handlerInfo.preferredAction = Ci.nsIHandlerInfo.useHelperApp;
-  handlerInfo.alwaysAskBeforeHandling = false;
-
-  // Say it's us:
-  let selfFile = Services.dirsvc.get("XREExeF", Ci.nsIFile);
-  // Make sure it's the .app
-  if (AppConstants.platform == "macosx") {
-    while (
-      !selfFile.leafName.endsWith(".app") &&
-      !selfFile.leafName.endsWith(".app/")
-    ) {
-      selfFile = selfFile.parent;
-    }
-  }
-  let selfHandlerApp = Cc[
-    "@mozilla.org/uriloader/local-handler-app;1"
-  ].createInstance(Ci.nsILocalHandlerApp);
-  selfHandlerApp.executable = selfFile;
-  handlerInfo.possibleApplicationHandlers.appendElement(selfHandlerApp);
-  handlerInfo.preferredApplicationHandler = selfHandlerApp;
-  handlerSvc.store(handlerInfo);
-
-  await BrowserTestUtils.withNewTab("about:blank", async browser => {
-    // Now, do some safety stubbing. If we do end up recursing we spawn
-    // infinite tabs. We definitely don't want that. Avoid it by stubbing
-    // our external URL handling bits:
-    let oldAddTab = gBrowser.addTab;
-    registerCleanupFunction(() => (gBrowser.addTab = oldAddTab));
-    let wrongThingHappenedPromise = new Promise(resolve => {
-      gBrowser.addTab = function(aURI) {
-        ok(false, "Tried to open unexpected URL in a tab: " + aURI);
-        resolve(null);
-        // Pass a dummy object to avoid upsetting BrowserContentHandler -
-        // if it thinks opening the tab failed, it tries to open a window instead,
-        // which we can't prevent as easily, and at which point we still end up
-        // with runaway tabs.
-        return {};
-      };
-    });
-
-    let askedUserPromise = BrowserTestUtils.domWindowOpenedAndLoaded();
-
-    info("Clicking a link that should open the unknown content type dialog");
-    await SpecialPowers.spawn(browser, [], () => {
-      let link = content.document.createElement("a");
-      link.download = "foo.foo";
-      link.textContent = "Foo file";
-      link.href = "data:application/x-foo,hello";
-      content.document.body.append(link);
-      link.click();
-    });
-    let dialog = await Promise.race([
-      wrongThingHappenedPromise,
-      askedUserPromise,
-    ]);
-    ok(dialog, "Should have gotten a dialog");
-    Assert.stringContains(
-      dialog.document.location.href,
-      "unknownContentType",
-      "Should have opened correct dialog."
-    );
-
-    let closePromise = BrowserTestUtils.windowClosed(dialog);
-    dialog.close();
-    await closePromise;
-    askedUserPromise = null;
-  });
-});

uriloader/exthandler/tests/mochitest/browser_protocolhandler_loop.js

@@ -44,9 +44,11 @@ add_task(async function test_helperapp() {
     // infinite tabs. We definitely don't want that. Avoid it by stubbing
     // our external URL handling bits:
     let oldAddTab = gBrowser.addTab;
-    registerCleanupFunction(() => (gBrowser.addTab = oldAddTab));
+    registerCleanupFunction(
+      () => (gBrowser.addTab = gBrowser.loadOneTab = oldAddTab)
+    );
     let wrongThingHappenedPromise = new Promise(resolve => {
-      gBrowser.addTab = function(aURI) {
+      gBrowser.addTab = gBrowser.loadOneTab = function(aURI) {
         ok(false, "Tried to open unexpected URL in a tab: " + aURI);
         resolve(null);
         // Pass a dummy object to avoid upsetting BrowserContentHandler -