Bug 1521729 - P2: Fix failure tests r=mayhemer

Due to the p1 patch, the first time we get the result back from NS_ShouldSecureUpgrade is always asynchronously.
This means that the first http request could be completed slower than the http requests opened after the first one. There are some tests affected by this change, since these tests assume that http requests should be completed in the same order as these requests were created.
This patch fixes the tests by introducing a new method in nsIHttpProtocolHandler. This method returns a promise and makes sure the HSTS data is ready to read synchronously when the promise is resolved.
Once the HSTS is ready to read, the order of open/close channels will be the same.

Differential Revision: https://phabricator.services.mozilla.com/D21683

--HG--
extra : moz-landing-system : lando

netwerk/ipc/NeckoParent.cpp

@@ -52,6 +52,7 @@
 #include "nsINetworkPredictor.h"
 #include "nsINetworkPredictorVerifier.h"
 #include "nsISpeculativeConnect.h"
+#include "nsHttpHandler.h"
 #include "nsNetUtil.h"
 
 using IPC::SerializedLoadContext;
@@ -976,5 +977,15 @@ mozilla::ipc::IPCResult NeckoParent::RecvInitSocketProcessBridge(
   return IPC_OK();
 }
 
+mozilla::ipc::IPCResult NeckoParent::RecvEnsureHSTSData(
+    EnsureHSTSDataResolver&& aResolver) {
+  auto callback = [aResolver{std::move(aResolver)}](bool aResult) {
+    aResolver(aResult);
+  };
+  gHttpHandler->EnsureHSTSDataReadyNative(
+      new HSTSDataCallbackWrapper(std::move(callback)));
+  return IPC_OK();
+}
+
 }  // namespace net
 }  // namespace mozilla

netwerk/ipc/NeckoParent.h

@@ -237,6 +237,9 @@ class NeckoParent : public PNeckoParent {
 
   mozilla::ipc::IPCResult RecvInitSocketProcessBridge(
       InitSocketProcessBridgeResolver&& aResolver);
+
+  mozilla::ipc::IPCResult RecvEnsureHSTSData(
+      EnsureHSTSDataResolver&& aResolver);
 };
 
 }  // namespace net

netwerk/ipc/PNecko.ipdl

@@ -144,6 +144,9 @@ parent:
   async InitSocketProcessBridge()
     returns (Endpoint<PSocketProcessBridgeChild> endpoint);
 
+  async EnsureHSTSData()
+    returns (bool result);
+
 child:
   /*
    * Bring up the http auth prompt for a nested remote mozbrowser.

netwerk/protocol/http/nsHttpHandler.cpp

@@ -48,6 +48,7 @@
 #include "nsPIDOMWindow.h"
 #include "nsINetworkLinkService.h"
 #include "nsHttpChannelAuthProvider.h"
+#include "nsNetUtil.h"
 #include "nsServiceManagerUtils.h"
 #include "nsComponentManagerUtils.h"
 #include "nsSocketTransportService2.h"
@@ -69,6 +70,7 @@
 
 #include "mozilla/dom/ContentParent.h"
 #include "mozilla/dom/Navigator.h"
+#include "mozilla/dom/Promise.h"
 #include "mozilla/dom/network/Connection.h"
 
 #include "nsNSSComponent.h"
@@ -131,6 +133,7 @@
 
 //-----------------------------------------------------------------------------
 
+using mozilla::dom::Promise;
 using mozilla::Telemetry::LABELS_NETWORK_HTTP_REDIRECT_TO_SCHEME;
 
 namespace mozilla {
@@ -2588,6 +2591,75 @@ nsHttpsHandler::AllowPort(int32_t aPort, const char *aScheme, bool *_retval) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsHttpHandler::EnsureHSTSDataReadyNative(HSTSDataCallbackWrapper *aCallback) {
+  MOZ_ASSERT(NS_IsMainThread());
+  MOZ_ASSERT(aCallback);
+
+  nsCOMPtr<nsIURI> uri;
+  nsresult rv = NS_NewURI(getter_AddRefs(uri), "http://example.com");
+  NS_ENSURE_SUCCESS(rv, rv);
+
+  bool shouldUpgrade = false;
+  bool willCallback = false;
+  OriginAttributes originAttributes;
+  RefPtr<HSTSDataCallbackWrapper> callback = aCallback;
+  auto func = [callback{std::move(callback)}](bool aResult, nsresult aStatus) {
+    callback->DoCallback(aResult);
+  };
+  rv = NS_ShouldSecureUpgrade(uri, nullptr, nullptr, false, false,
+                              originAttributes, shouldUpgrade, std::move(func),
+                              willCallback);
+  if (NS_FAILED(rv) || !willCallback) {
+    aCallback->DoCallback(false);
+    return rv;
+  }
+
+  return rv;
+}
+
+NS_IMETHODIMP
+nsHttpHandler::EnsureHSTSDataReady(JSContext *aCx, Promise **aPromise) {
+  if (NS_WARN_IF(!aCx)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  nsIGlobalObject *globalObject = xpc::CurrentNativeGlobal(aCx);
+  if (NS_WARN_IF(!globalObject)) {
+    return NS_ERROR_FAILURE;
+  }
+
+  ErrorResult result;
+  RefPtr<Promise> promise = Promise::Create(globalObject, result);
+  if (NS_WARN_IF(result.Failed())) {
+    return result.StealNSResult();
+  }
+
+  if (IsNeckoChild()) {
+    gNeckoChild->SendEnsureHSTSData()->Then(
+        GetMainThreadSerialEventTarget(), __func__,
+        [promise(promise)](
+            NeckoChild::EnsureHSTSDataPromise::ResolveOrRejectValue &&aResult) {
+          if (aResult.IsResolve()) {
+            promise->MaybeResolve(aResult.ResolveValue());
+          } else {
+            promise->MaybeReject(NS_ERROR_FAILURE);
+          }
+        });
+    promise.forget(aPromise);
+    return NS_OK;
+  }
+
+  auto callback = [promise(promise)](bool aResult) {
+    promise->MaybeResolve(aResult);
+  };
+
+  RefPtr<HSTSDataCallbackWrapper> wrapper =
+      new HSTSDataCallbackWrapper(std::move(callback));
+  promise.forget(aPromise);
+  return EnsureHSTSDataReadyNative(wrapper);
+}
+
 void nsHttpHandler::ShutdownConnectionManager() {
   // ensure connection manager is shutdown
   if (mConnMgr) {

netwerk/protocol/http/nsHttpHandler.h

@@ -6,6 +6,8 @@
 #ifndef nsHttpHandler_h__
 #define nsHttpHandler_h__
 
+#include <functional>
+
 #include "nsHttp.h"
 #include "nsHttpAuthCache.h"
 #include "nsHttpConnectionMgr.h"
@@ -754,6 +756,32 @@ class nsHttpsHandler : public nsIHttpProtocolHandler,
   MOZ_MUST_USE nsresult Init();
 };
 
+//-----------------------------------------------------------------------------
+// HSTSDataCallbackWrapper - A threadsafe helper class to wrap the callback.
+//
+// We need this because dom::promise and EnsureHSTSDataResolver are not
+// threadsafe.
+//-----------------------------------------------------------------------------
+class HSTSDataCallbackWrapper final {
+ public:
+  NS_INLINE_DECL_THREADSAFE_REFCOUNTING(HSTSDataCallbackWrapper)
+
+  explicit HSTSDataCallbackWrapper(std::function<void(bool)> &&aCallback)
+      : mCallback(std::move(aCallback)) {
+    MOZ_ASSERT(NS_IsMainThread());
+  }
+
+  void DoCallback(bool aResult) {
+    MOZ_ASSERT(NS_IsMainThread());
+    mCallback(aResult);
+  }
+
+ private:
+  ~HSTSDataCallbackWrapper() = default;
+
+  std::function<void(bool)> mCallback;
+};
+
 }  // namespace net
 }  // namespace mozilla
 

netwerk/protocol/http/nsIHttpProtocolHandler.idl

@@ -5,6 +5,15 @@
 
 #include "nsIProxiedProtocolHandler.idl"
 
+%{C++
+namespace mozilla {
+namespace net {
+class HSTSDataCallbackWrapper;
+}
+}
+%}
+[ptr] native HSTSDataCallbackWrapperPtr(mozilla::net::HSTSDataCallbackWrapper);
+
 [scriptable, uuid(c48126d9-2ddd-485b-a51a-378e917e75f8)]
 interface nsIHttpProtocolHandler : nsIProxiedProtocolHandler
 {
@@ -47,6 +56,20 @@ interface nsIHttpProtocolHandler : nsIProxiedProtocolHandler
      */
     [must_use] readonly attribute ACString misc;
 
+    /**
+     * This function is used to ensure HSTS data storage is ready to read after
+     * the returned promise is resolved.
+     * Note that this function should only used for testing.
+     * See bug 1521729 for more details.
+     */
+    [implicit_jscontext]
+    Promise EnsureHSTSDataReady();
+
+    /**
+     * A C++ friendly version of EnsureHSTSDataReady
+     */
+    [noscript]
+    void EnsureHSTSDataReadyNative(in HSTSDataCallbackWrapperPtr aCallback);
 };
 
 %{C++

netwerk/test/unit/test_bug596443.js

@@ -1,6 +1,9 @@
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
+
 var httpserver = new HttpServer();
 
 var expectedOnStopRequests = 3;
@@ -61,14 +64,16 @@ function run_test() {
     // clear cache
     evict_cache_entries();
 
-    var ch0 = setupChannel("/bug596443", "Response0", Ci.nsIRequest.LOAD_BYPASS_CACHE);
-    ch0.asyncOpen(new Listener("Response0"));
+    httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+        var ch0 = setupChannel("/bug596443", "Response0", Ci.nsIRequest.LOAD_BYPASS_CACHE);
+        ch0.asyncOpen(new Listener("Response0"));
 
-    var ch1 = setupChannel("/bug596443", "Response1", Ci.nsIRequest.LOAD_BYPASS_CACHE);
-    ch1.asyncOpen(new Listener("Response1"));
+        var ch1 = setupChannel("/bug596443", "Response1", Ci.nsIRequest.LOAD_BYPASS_CACHE);
+        ch1.asyncOpen(new Listener("Response1"));
 
-    var ch2 = setupChannel("/bug596443", "Should not be used");
-    ch2.asyncOpen(new Listener("Response1")); // Note param: we expect this to come from cache
+        var ch2 = setupChannel("/bug596443", "Should not be used");
+        ch2.asyncOpen(new Listener("Response1")); // Note param: we expect this to come from cache
+    });
 
     do_test_pending();
 }

netwerk/test/unit/test_cache2-29a-concurrent_read_resumable_entry_size_zero.js

@@ -14,6 +14,8 @@ This test is using a resumable response.
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
 
 XPCOMUtils.defineLazyGetter(this, "URL", function() {
   return "http://localhost:" + httpServer.identity.primaryPort;
@@ -52,10 +54,12 @@ function run_test()
   httpServer.registerPathHandler("/content", contentHandler);
   httpServer.start(-1);
 
-  var chan1 = make_channel(URL + "/content");
-  chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
-  var chan2 = make_channel(URL + "/content");
-  chan2.asyncOpen(new ChannelListener(secondTimeThrough, null));
+  httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+    var chan1 = make_channel(URL + "/content");
+    chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
+    var chan2 = make_channel(URL + "/content");
+    chan2.asyncOpen(new ChannelListener(secondTimeThrough, null));
+  });
 
   do_test_pending();
 }

netwerk/test/unit/test_cache2-29b-concurrent_read_non-resumable_entry_size_zero.js

@@ -16,6 +16,9 @@ This test is using a non-resumable response.
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
+
 XPCOMUtils.defineLazyGetter(this, "URL", function() {
   return "http://localhost:" + httpServer.identity.primaryPort;
 });
@@ -51,10 +54,12 @@ function run_test()
   httpServer.registerPathHandler("/content", contentHandler);
   httpServer.start(-1);
 
-  var chan1 = make_channel(URL + "/content");
-  chan1.asyncOpen(new ChannelListener(firstTimeThrough, null, CL_ALLOW_UNKNOWN_CL));
-  var chan2 = make_channel(URL + "/content");
-  chan2.asyncOpen(new ChannelListener(secondTimeThrough, null, CL_ALLOW_UNKNOWN_CL));
+  httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+    var chan1 = make_channel(URL + "/content");
+    chan1.asyncOpen(new ChannelListener(firstTimeThrough, null, CL_ALLOW_UNKNOWN_CL));
+    var chan2 = make_channel(URL + "/content");
+    chan2.asyncOpen(new ChannelListener(secondTimeThrough, null, CL_ALLOW_UNKNOWN_CL));
+  });
 
   do_test_pending();
 }

netwerk/test/unit/test_cache2-29c-concurrent_read_half-interrupted.js

@@ -16,6 +16,8 @@ This test is using a resumable response.
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
 
 XPCOMUtils.defineLazyGetter(this, "URL", function() {
   return "http://localhost:" + httpServer.identity.primaryPort;
@@ -71,10 +73,12 @@ function run_test()
   httpServer.registerPathHandler("/content", contentHandler);
   httpServer.start(-1);
 
-  var chan1 = make_channel(URL + "/content");
-  chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
-  var chan2 = make_channel(URL + "/content");
-  chan2.asyncOpen(new ChannelListener(secondTimeThrough, null));
+  httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+    var chan1 = make_channel(URL + "/content");
+    chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
+    var chan2 = make_channel(URL + "/content");
+    chan2.asyncOpen(new ChannelListener(secondTimeThrough, null));
+  });
 
   do_test_pending();
 }

netwerk/test/unit/test_cache2-29d-concurrent_read_half-corrupted-206.js

@@ -17,6 +17,8 @@ This test is using a resumable response.
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
 
 XPCOMUtils.defineLazyGetter(this, "URL", function() {
   return "http://localhost:" + httpServer.identity.primaryPort;
@@ -71,10 +73,12 @@ function run_test()
   httpServer.registerPathHandler("/content", contentHandler);
   httpServer.start(-1);
 
-  var chan1 = make_channel(URL + "/content");
-  chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
-  var chan2 = make_channel(URL + "/content");
-  chan2.asyncOpen(new ChannelListener(secondTimeThrough, null, CL_EXPECT_FAILURE));
+  httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+    var chan1 = make_channel(URL + "/content");
+    chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
+    var chan2 = make_channel(URL + "/content");
+    chan2.asyncOpen(new ChannelListener(secondTimeThrough, null, CL_EXPECT_FAILURE));
+  });
 
   do_test_pending();
 }

netwerk/test/unit/test_cache2-29e-concurrent_read_half-non-206-response.js

@@ -17,6 +17,8 @@ This test is using a resumable response.
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
 
 XPCOMUtils.defineLazyGetter(this, "URL", function() {
   return "http://localhost:" + httpServer.identity.primaryPort;
@@ -66,10 +68,12 @@ function run_test()
   httpServer.registerPathHandler("/content", contentHandler);
   httpServer.start(-1);
 
-  var chan1 = make_channel(URL + "/content");
-  chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
-  var chan2 = make_channel(URL + "/content");
-  chan2.asyncOpen(new ChannelListener(secondTimeThrough, null, CL_EXPECT_FAILURE));
+  httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+    var chan1 = make_channel(URL + "/content");
+    chan1.asyncOpen(new ChannelListener(firstTimeThrough, null));
+    var chan2 = make_channel(URL + "/content");
+    chan2.asyncOpen(new ChannelListener(secondTimeThrough, null, CL_EXPECT_FAILURE));
+  });
 
   do_test_pending();
 }

netwerk/test/unit/test_channel_close.js

@@ -1,6 +1,9 @@
 const {HttpServer} = ChromeUtils.import("resource://testing-common/httpd.js");
 const {NetUtil} = ChromeUtils.import("resource://gre/modules/NetUtil.jsm");
 
+var httpProtocolHandler = Cc["@mozilla.org/network/protocol;1?name=http"]
+                          .getService(Ci.nsIHttpProtocolHandler);
+
 XPCOMUtils.defineLazyGetter(this, "URL", function() {
   return "http://localhost:" + httpserver.identity.primaryPort;
 });
@@ -15,21 +18,23 @@ function run_test() {
   httpserver.registerPathHandler(testpath, serverHandler);
   httpserver.start(-1);
 
-  var local_channel;
+  httpProtocolHandler.EnsureHSTSDataReady().then(function() {
+    var local_channel;
 
-  // Opened channel that has no remaining references on shutdown
-  local_channel = setupChannel(testpath);
-  local_channel.asyncOpen(new ChannelListener(checkRequest, local_channel));
+    // Opened channel that has no remaining references on shutdown
+    local_channel = setupChannel(testpath);
+    local_channel.asyncOpen(new ChannelListener(checkRequest, local_channel));
 
-  // Opened channel that has no remaining references after being opened
-  setupChannel(testpath).asyncOpen(new ChannelListener(function() {}, null));
-  
-  // Unopened channel that has remaining references on shutdown
-  live_channels.push(setupChannel(testpath));
+    // Opened channel that has no remaining references after being opened
+    setupChannel(testpath).asyncOpen(new ChannelListener(function() {}, null));
 
-  // Opened channel that has remaining references on shutdown
-  live_channels.push(setupChannel(testpath));
-  live_channels[1].asyncOpen(new ChannelListener(checkRequestFinish, live_channels[1]));
+    // Unopened channel that has remaining references on shutdown
+    live_channels.push(setupChannel(testpath));
+
+    // Opened channel that has remaining references on shutdown
+    live_channels.push(setupChannel(testpath));
+    live_channels[1].asyncOpen(new ChannelListener(checkRequestFinish, live_channels[1]));
+  });
 
   do_test_pending();
 }