Bug 1321878 - Disable diagnostic assert and make test less racy. r=me, a=gchang
authorValentin Gosu <valentin.gosu@gmail.com>
Thu, 02 Feb 2017 15:03:32 +0100
changeset 378992 5e211036864b9ed718084ece95d6a64488c976c1
parent 378991 84c5b75e91d816c74375215208f98b9487836fde
child 378993 05659f9e9b7b94bde3af270f2f301f4edea4d62e
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersme, gchang
bugs1321878
milestone53.0
Bug 1321878 - Disable diagnostic assert and make test less racy. r=me, a=gchang MozReview-Commit-ID: LKiseNvd7t2
netwerk/protocol/http/HttpChannelChild.cpp
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/service-workers/service-worker/performance-timeline.https.html
--- a/netwerk/protocol/http/HttpChannelChild.cpp
+++ b/netwerk/protocol/http/HttpChannelChild.cpp
@@ -900,22 +900,22 @@ HttpChannelChild::OnStopRequest(const ns
   mTransactionTimings.domainLookupStart = timing.domainLookupStart;
   mTransactionTimings.domainLookupEnd = timing.domainLookupEnd;
   mTransactionTimings.connectStart = timing.connectStart;
   mTransactionTimings.connectEnd = timing.connectEnd;
   mTransactionTimings.requestStart = timing.requestStart;
   mTransactionTimings.responseStart = timing.responseStart;
   mTransactionTimings.responseEnd = timing.responseEnd;
 
-  // Do not overwrite or adjust the original mAsyncOpenTime.  We must use the
-  // original child process time in order to account for child side work and IPC
-  // transit overhead.  This depends on TimeStamp being equivalent across
-  // processes.  We work hard to ensure this on modern hardware, but there could
-  // be some variance on older devices.
-  MOZ_DIAGNOSTIC_ASSERT(mAsyncOpenTime <= timing.fetchStart);
+  // Do not overwrite or adjust the original mAsyncOpenTime by timing.fetchStart
+  // We must use the original child process time in order to account for child
+  // side work and IPC transit overhead.
+  // XXX: This depends on TimeStamp being equivalent across processes.
+  // This is true for modern hardware but for older platforms it is not always
+  // true.
 
   mRedirectStartTimeStamp = timing.redirectStart;
   mRedirectEndTimeStamp = timing.redirectEnd;
   mTransferSize = timing.transferSize;
   mEncodedBodySize = timing.encodedBodySize;
   mProtocolVersion = timing.protocolVersion;
 
   mCacheReadStart = timing.cacheReadStart;
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -59611,16 +59611,21 @@
      {}
     ]
    ],
    "service-workers/service-worker/resources/dummy.txt": [
     [
      {}
     ]
    ],
+   "service-workers/service-worker/resources/empty-but-slow-worker.js": [
+    [
+     {}
+    ]
+   ],
    "service-workers/service-worker/resources/empty-worker.js": [
     [
      {}
     ]
    ],
    "service-workers/service-worker/resources/empty.js": [
     [
      {}
@@ -196168,17 +196173,17 @@
    "7c397361b27e4c4d90a84d92b16593ac40d443a9",
    "testharness"
   ],
   "service-workers/service-worker/oninstall-script-error.https.html": [
    "0497bf37f0e3b55a6a4745cae2ec700b6f963fd3",
    "testharness"
   ],
   "service-workers/service-worker/performance-timeline.https.html": [
-   "4e9fda9c0fa9b1c996c5053da4139a038b70cc39",
+   "23d9e3dc830b83370875387cd5d6e1d5e913452f",
    "testharness"
   ],
   "service-workers/service-worker/postmessage-msgport-to-client.https.html": [
    "21ae7fb96321dc75e7063c27b7e6838e6b9ff6c6",
    "testharness"
   ],
   "service-workers/service-worker/postmessage-to-client.https.html": [
    "8a96689891c9e49456c48e47a09405fe2edb4cce",
@@ -196327,16 +196332,20 @@
   "service-workers/service-worker/resources/dummy.html": [
    "b15855240cf7dbc5df4ee30a544d7ff94935d146",
    "support"
   ],
   "service-workers/service-worker/resources/dummy.txt": [
    "33ab5639bfd8e7b95eb1d8d0b87781d4ffea4d5d",
    "support"
   ],
+  "service-workers/service-worker/resources/empty-but-slow-worker.js": [
+   "45db58024c3207922c5c03ff92636eb829ef382e",
+   "support"
+  ],
   "service-workers/service-worker/resources/empty-worker.js": [
    "84b3339c3419e318803e51f46d7252d9e8ac183b",
    "support"
   ],
   "service-workers/service-worker/resources/empty.js": [
    "da39a3ee5e6b4b0d3255bfef95601890afd80709",
    "support"
   ],
--- a/testing/web-platform/tests/service-workers/service-worker/performance-timeline.https.html
+++ b/testing/web-platform/tests/service-workers/service-worker/performance-timeline.https.html
@@ -16,37 +16,31 @@ promise_test(t => {
   let url = new URL(scope, window.location).href;
   let slowURL = url + '&slow';
   let frame;
   return service_worker_unregister_and_register(t, script, scope)
     .then(reg => wait_for_state(t, reg.installing, 'activated'))
     .then(_ => with_iframe(scope))
     .then(f => {
       frame = f;
-      return Promise.all([
-        // This will get effectively an empty service worker FetchEvent
-        // handler.  It should have no additional delay.  Note that the
-        // text() call is necessary to complete the response and have the
-        // timings show up in the performance entries.
-        frame.contentWindow.fetch(url).then(r => r && r.text()),
-        // This will cause the service worker to spin for two seconds
-        // in its FetchEvent handler.
-        frame.contentWindow.fetch(slowURL).then(r => r && r.text())
-      ]);
+      return frame.contentWindow.fetch(url).then(r => r && r.text());
+    })
+    .then(_ => {
+      return frame.contentWindow.fetch(slowURL).then(r => r && r.text());
     })
     .then(_ => {
       function elapsed(u) {
         let entry = frame.contentWindow.performance.getEntriesByName(u);
         return entry[0] ? entry[0].duration : undefined;
       }
       let urlTime = elapsed(url);
       let slowURLTime = elapsed(slowURL);
       // Verify the request slowed by the service worker is indeed measured
       // to be slower.  Note, we compare to smaller delay instead of the exact
       // delay amount to avoid making the test racy under automation.
-      assert_true(slowURLTime >= urlTime + 1500,
+      assert_greater_than(slowURLTime, urlTime + 1000,
                   'Slow service worker request should measure increased delay.');
       frame.remove();
       return service_worker_unregister_and_done(t, scope);
     })
 }, 'empty service worker fetch event included in performance timings');
 
 </script>