|Submitter||Diff||Changes||Open Issues||Last Updated|
|Error loading review requests:|
58 bytes, text/x-review-board-request
|Details | Review|
While investigating a Talos regression (bug 1289410), I noticed that our e10s DAMP runs aren't quite correct. We load two pages during a test run, the simple page (chrome://damp/content/pages/simple.html) and complicated page (http://localhost:56620/tests/tp5n/bild.de/www.bild.de/index.html). Since the simple page uses a chrome:// URL, it will be loaded in the main process by default. However, we want all test pages to be in remote browsers for the test to accurately DevTools in the wild with an e10s enabled browser. Fixing this will invalidate past test results on e10s.
a year ago
Created attachment 8777487 [details] Bug 1291809 - Use http URL for DAMP simple page to get remote browser. Review commit: https://reviewboard.mozilla.org/r/69016/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/69016/
Comment on attachment 8777487 [details] Bug 1291809 - Use http URL for DAMP simple page to get remote browser. https://reviewboard.mozilla.org/r/69016/#review66116
Looking at the results, the changes are kind of all over the map... In general, the complicated page with e10s off is unchanged. The remaining results have changed a lot: the simple page has changed for both e10s off and on, and the complicated page has changed for e10s on. The change to complicated page with e10s on is a bit surprising since we've only change the simple page here. My best guess is that since the simple page loads first, and it now goes over HTTP, this affects network caches and other platform timings when we load another HTTP page for the complicated portion. I have confirmed with local logging that we're correctly getting a remote browser from the simple page like we expect, so it's probably best to accept these as the new baseline and not try to read much into why they've changed.
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #4) > I have confirmed with local logging that we're correctly getting a remote > browser from the simple page like we expect, so it's probably best to accept > these as the new baseline and not try to read much into why they've changed. That works for me
Comment on attachment 8777487 [details] Bug 1291809 - Use http URL for DAMP simple page to get remote browser. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/69016/diff/1-2/
Updated commit with signed add-on.
Pushed by email@example.com: https://hg.mozilla.org/integration/autoland/rev/d97d3eb03261 Use http URL for DAMP simple page to get remote browser. r=bgrins
here are the damp changes for this: https://treeherder.mozilla.org/perf.html#/alerts?id=2250 looks like: non-e10s regressed slightly e10s improved slightly if this wasn't the expected outcome, lets fix it- otherwise, lets accept it!
(In reply to Joel Maher ( :jmaher ) from comment #10) > here are the damp changes for this: > https://treeherder.mozilla.org/perf.html#/alerts?id=2250 > > looks like: > non-e10s regressed slightly > e10s improved slightly > > if this wasn't the expected outcome, lets fix it- otherwise, lets accept it! This matches the results in the try run: https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=451e84cd0ff9cb2f3090b495730413494795374a&newProject=try&newRevision=02f43928bc94fd005c6ce36ca211235ad5d7b339&framework=1&filter=damp&showOnlyImportant=0 As I said in comment 4, I don't have a strong intuition for why the results shake out like this... but we've made a major change to the test, and it's now doing what we want it to do, so we should accept the change. Thanks for the follow up!