bug 797684 - http-on-modify-request can reflect ProxyInfo r=biesi
authorPatrick McManus <mcmanus@ducksong.com>
Mon, 08 Oct 2012 20:19:08 -0400
changeset 109710 6ee4101a85f34f239230546ab546d54078c4b0eb
parent 109709 d7650d584078bb923d787a18487088998f7d7843
child 109711 4a6c1725dd4becbd22e74b378766506fe97d45f9
push id23648
push useremorley@mozilla.com
push dateTue, 09 Oct 2012 14:23:49 +0000
treeherdermozilla-central@dd61540f237c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbiesi
bugs797684
milestone19.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
bug 797684 - http-on-modify-request can reflect ProxyInfo r=biesi
netwerk/protocol/http/nsHttpChannel.cpp
netwerk/test/unit/test_protocolproxyservice.js
--- a/netwerk/protocol/http/nsHttpChannel.cpp
+++ b/netwerk/protocol/http/nsHttpChannel.cpp
@@ -4296,35 +4296,32 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
     // Remember the cookie header that was set, if any
     const char *cookieHeader = mRequestHead.PeekHeader(nsHttp::Cookie);
     if (cookieHeader) {
         mUserSetCookieHeader = cookieHeader;
     }
 
     AddCookiesToRequest();
 
-    // notify "http-on-modify-request" observers
-    gHttpHandler->OnModifyRequest(this);
-
     mIsPending = true;
     mWasOpened = true;
 
     mListener = listener;
     mListenerContext = context;
 
     // add ourselves to the load group.  from this point forward, we'll report
     // all failures asynchronously.
     if (mLoadGroup)
         mLoadGroup->AddRequest(this, nullptr);
 
-    // Collect mAsyncOpenTime after we have called all observers like
-    // "http-on-modify-request" and load group observers that may set
-    // mTimingEnabled flag.
-    if (mTimingEnabled)
-        mAsyncOpenTime = mozilla::TimeStamp::Now();
+    // record asyncopen time unconditionally and clear it if we
+    // don't want it after OnModifyRequest() weighs in. But waiting for
+    // that to complete would mean we don't include proxy resolution in the
+    // timing.
+    mAsyncOpenTime = mozilla::TimeStamp::Now();
 
     // the only time we would already know the proxy information at this
     // point would be if we were proxying a non-http protocol like ftp
     if (!mProxyInfo && NS_SUCCEEDED(ResolveProxy()))
         return NS_OK;
 
     rv = BeginConnect();
     if (NS_FAILED(rv))
@@ -4334,16 +4331,24 @@ nsHttpChannel::AsyncOpen(nsIStreamListen
 }
 
 nsresult
 nsHttpChannel::BeginConnect()
 {
     LOG(("nsHttpChannel::BeginConnect [this=%p]\n", this));
     nsresult rv;
 
+    // notify "http-on-modify-request" observers
+    gHttpHandler->OnModifyRequest(this);
+
+    // If mTimingEnabled flag is not set after OnModifyRequest() then
+    // clear the already recorded AsyncOpen value for consistency.
+    if (!mTimingEnabled)
+        mAsyncOpenTime = mozilla::TimeStamp();
+
     // Construct connection info object
     nsAutoCString host;
     int32_t port = -1;
     bool usingSSL = false;
 
     rv = mURI->SchemeIs("https", &usingSSL);
     if (NS_SUCCEEDED(rv))
         rv = mURI->GetAsciiHost(host);
@@ -4514,21 +4519,20 @@ nsHttpChannel::OnProxyAvailable(nsICance
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsIProxiedChannel
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
 nsHttpChannel::GetProxyInfo(nsIProxyInfo **result)
 {
     if (!mConnectionInfo)
-        *result = nullptr;
-    else {
+        *result = mProxyInfo;
+    else
         *result = mConnectionInfo->ProxyInfo();
-        NS_IF_ADDREF(*result);
-    }
+    NS_IF_ADDREF(*result);
     return NS_OK;
 }
 
 //-----------------------------------------------------------------------------
 // nsHttpChannel::nsITimedChannel
 //-----------------------------------------------------------------------------
 
 NS_IMETHODIMP
--- a/netwerk/test/unit/test_protocolproxyservice.js
+++ b/netwerk/test/unit/test_protocolproxyservice.js
@@ -623,30 +623,53 @@ function failed_script_callback(pi)
   // we should go direct
   do_check_eq(pi, null);
 
   // test that we honor filters when configured to go direct
   prefs.setIntPref("network.proxy.type", 0);
   directFilter = new TestFilter("http", "127.0.0.1", 7246, 0, 0);
   pps.registerFilter(directFilter, 10);
 
+  // test that on-modify-request contains the proxy info too
+  var obs = Components.classes["@mozilla.org/observer-service;1"].getService();
+  obs = obs.QueryInterface(Components.interfaces.nsIObserverService);
+  obs.addObserver(directFilterListener, "http-on-modify-request", false);
+
   var chan = ios.newChannel("http://127.0.0.1:7247", "", null);
   chan.asyncOpen(directFilterListener, chan);
 }
 
 var directFilterListener = {
+  onModifyRequestCalled : false,
+
   onStartRequest: function test_onStart(request, ctx) {  },
   onDataAvailable: function test_OnData() { },
 
   onStopRequest: function test_onStop(request, ctx, status) {
+    // check on the PI from the channel itself
     ctx.QueryInterface(Components.interfaces.nsIProxiedChannel);
     check_proxy(ctx.proxyInfo, "http", "127.0.0.1", 7246, 0, 0, false);
     pps.unregisterFilter(directFilter);
+
+    // check on the PI from on-modify-request
+    do_check_true(this.onModifyRequestCalled);
+    var obs = Components.classes["@mozilla.org/observer-service;1"].getService();
+    obs = obs.QueryInterface(Components.interfaces.nsIObserverService);
+    obs.removeObserver(this, "http-on-modify-request");
     do_test_finished();
   },
+
+   observe: function(subject, topic, data) {
+     if (topic === "http-on-modify-request" &&
+         subject instanceof Components.interfaces.nsIHttpChannel &&
+         subject instanceof Components.interfaces.nsIProxiedChannel) {
+       check_proxy(subject.proxyInfo, "http", "127.0.0.1", 7246, 0, 0, false);
+       this.onModifyRequestCalled = true;
+     }
+   }
 };
 
 function run_deprecated_sync_test()
 {
   var uri = ios.newURI("http://www.mozilla.org/", null, null);
 
   pps.QueryInterface(Components.interfaces.nsIProtocolProxyService2);