Re: PSA - With e10s enabled, initial browser in new windows will soon be remote by default
Hey bkelly, Do we still kill the child process if the last content tab is closed? Yes we do. When a TabParent gets destroyed, it calls into here: http://searchfox.org/mozilla-central/rev/336105a0de49f41a2cc203db7b7ee7266d0d558b/dom/ipc/ContentParent.cpp#2074 That code checks to see if any other tabs exist for the destroyed tabs content process. If not, the content process is marked for destruction. -Mike On 25 July 2016 at 10:44, Ben Kelly wrote: > Mike, > > Do we still kill the child process if the last content tab is closed? For > example, close all tabs in all windows except for an about:memory tab or > something. Or do we keep the child process alive as long as the > browser.xul window is alive? > > Keeping the process alive would actually make a few things easier with our > current service worker/push/devtools setup. > > Thanks. > > Ben > > On Mon, Jul 25, 2016 at 10:29 AM, Mike Conley wrote: > >> After much toil, this has finally landed on central. Hopefully it sticks. >> >> Big thanks to all of my reviewers - especially smaug, who reviewed the >> bulk >> of the weirder stuff. >> >> -Mike >> >> On 9 June 2016 at 14:21, Mike Conley wrote: >> >> > If you don’t write tests that open browser windows or work on the >> > front-end of Firefox, you can probably ignore this. >> > >> > The initial browser in browser.xul is non-remote by default. This means >> > that as soon as it browses to some URL that can be loaded in the content >> > process (about:home, for example), we kick off the remoteness flipping >> > machinery that swaps out that initial browser and puts a remote one >> down in >> > its place to load the content in the child process. >> > >> > In bug 1261842, I’m working on switching things around so that the >> initial >> > browser is remote as soon as possible (there’s still some set-up >> required >> > in JS, so I can’t just add the remote attribute to that initial >> browser). >> > Doing the flip sooner means we don’t have to set up the content viewer >> for >> > the non-remote browser (which gets immediately destroyed after the >> > remoteness flip), which is good for window-opening performance. >> > >> > Normally, I wouldn’t bother these two mailing lists with this kind of >> > information, but while working on these patches, I’ve had to fix a >> bunch of >> > tests that worked properly under the old assumption (initial browser is >> > non-remote) but not under the new one (initial browser is remote), and I >> > wanted to give a PSA on those writing tests that open windows. >> > >> > Here’s the deal: If you’re writing browser mochitests and you need to >> open >> > a window, please use BrowserTestUtils.openNewBrowserWindow. If you need >> to >> > trigger window openings via another mechanism (like window.open in >> > content), but need to do things with the newly opened window, set up a >> > Promise with BrowserTestUtils.waitForNewWindow. Both of these methods >> are >> > doing the right thing in terms of waiting for the new window to set >> itself >> > up and for the initial browser to be ready to do things with it. >> > >> > If you care - here’s why: without them, you run the risk of falling into >> > the mistake I was seeing in a number of tests, which was opening a new >> > window, waiting for its load event, telling the initial browser to load >> > some URI, and then waiting for BrowserTestUtils.browserLoaded to resolve >> > before proceeding. This technique will fail frequently, because >> sometimes >> > the message to load the URI will arrive at content after it has loaded >> its >> > initial about:blank, but before the parent has had an opportunity to >> hear >> > that the initial about:blank has been loaded. This means that the >> > browserLoaded Promise will sometimes resolve even though the URI you >> > requested hasn’t finished (or maybe even started) loading. >> > >> > Anyhow, with the patches in the bug, I appear to have fixed all of the >> > tests that fall into this trap - but if you write a new test that’s >> opening >> > a window that’s failing frequently and you don’t know why, it might be >> > because you need to use those BrowserTestUtils methods. >> > >> > Feel free to follow up with me in this thread or over IRC if there are >> any >> > questions or concerns regarding bug 1261842. >> > >> > Thanks, >> > >> > -Mike >> > >> ___ >> dev-platform mailing list >> dev-platform@lists.mozilla.org >> https://lists.mozilla.org/listinfo/dev-platform >> > > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA - With e10s enabled, initial browser in new windows will soon be remote by default
Mike, Do we still kill the child process if the last content tab is closed? For example, close all tabs in all windows except for an about:memory tab or something. Or do we keep the child process alive as long as the browser.xul window is alive? Keeping the process alive would actually make a few things easier with our current service worker/push/devtools setup. Thanks. Ben On Mon, Jul 25, 2016 at 10:29 AM, Mike Conley wrote: > After much toil, this has finally landed on central. Hopefully it sticks. > > Big thanks to all of my reviewers - especially smaug, who reviewed the bulk > of the weirder stuff. > > -Mike > > On 9 June 2016 at 14:21, Mike Conley wrote: > > > If you don’t write tests that open browser windows or work on the > > front-end of Firefox, you can probably ignore this. > > > > The initial browser in browser.xul is non-remote by default. This means > > that as soon as it browses to some URL that can be loaded in the content > > process (about:home, for example), we kick off the remoteness flipping > > machinery that swaps out that initial browser and puts a remote one down > in > > its place to load the content in the child process. > > > > In bug 1261842, I’m working on switching things around so that the > initial > > browser is remote as soon as possible (there’s still some set-up required > > in JS, so I can’t just add the remote attribute to that initial browser). > > Doing the flip sooner means we don’t have to set up the content viewer > for > > the non-remote browser (which gets immediately destroyed after the > > remoteness flip), which is good for window-opening performance. > > > > Normally, I wouldn’t bother these two mailing lists with this kind of > > information, but while working on these patches, I’ve had to fix a bunch > of > > tests that worked properly under the old assumption (initial browser is > > non-remote) but not under the new one (initial browser is remote), and I > > wanted to give a PSA on those writing tests that open windows. > > > > Here’s the deal: If you’re writing browser mochitests and you need to > open > > a window, please use BrowserTestUtils.openNewBrowserWindow. If you need > to > > trigger window openings via another mechanism (like window.open in > > content), but need to do things with the newly opened window, set up a > > Promise with BrowserTestUtils.waitForNewWindow. Both of these methods are > > doing the right thing in terms of waiting for the new window to set > itself > > up and for the initial browser to be ready to do things with it. > > > > If you care - here’s why: without them, you run the risk of falling into > > the mistake I was seeing in a number of tests, which was opening a new > > window, waiting for its load event, telling the initial browser to load > > some URI, and then waiting for BrowserTestUtils.browserLoaded to resolve > > before proceeding. This technique will fail frequently, because sometimes > > the message to load the URI will arrive at content after it has loaded > its > > initial about:blank, but before the parent has had an opportunity to hear > > that the initial about:blank has been loaded. This means that the > > browserLoaded Promise will sometimes resolve even though the URI you > > requested hasn’t finished (or maybe even started) loading. > > > > Anyhow, with the patches in the bug, I appear to have fixed all of the > > tests that fall into this trap - but if you write a new test that’s > opening > > a window that’s failing frequently and you don’t know why, it might be > > because you need to use those BrowserTestUtils methods. > > > > Feel free to follow up with me in this thread or over IRC if there are > any > > questions or concerns regarding bug 1261842. > > > > Thanks, > > > > -Mike > > > ___ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: PSA - With e10s enabled, initial browser in new windows will soon be remote by default
After much toil, this has finally landed on central. Hopefully it sticks. Big thanks to all of my reviewers - especially smaug, who reviewed the bulk of the weirder stuff. -Mike On 9 June 2016 at 14:21, Mike Conley wrote: > If you don’t write tests that open browser windows or work on the > front-end of Firefox, you can probably ignore this. > > The initial browser in browser.xul is non-remote by default. This means > that as soon as it browses to some URL that can be loaded in the content > process (about:home, for example), we kick off the remoteness flipping > machinery that swaps out that initial browser and puts a remote one down in > its place to load the content in the child process. > > In bug 1261842, I’m working on switching things around so that the initial > browser is remote as soon as possible (there’s still some set-up required > in JS, so I can’t just add the remote attribute to that initial browser). > Doing the flip sooner means we don’t have to set up the content viewer for > the non-remote browser (which gets immediately destroyed after the > remoteness flip), which is good for window-opening performance. > > Normally, I wouldn’t bother these two mailing lists with this kind of > information, but while working on these patches, I’ve had to fix a bunch of > tests that worked properly under the old assumption (initial browser is > non-remote) but not under the new one (initial browser is remote), and I > wanted to give a PSA on those writing tests that open windows. > > Here’s the deal: If you’re writing browser mochitests and you need to open > a window, please use BrowserTestUtils.openNewBrowserWindow. If you need to > trigger window openings via another mechanism (like window.open in > content), but need to do things with the newly opened window, set up a > Promise with BrowserTestUtils.waitForNewWindow. Both of these methods are > doing the right thing in terms of waiting for the new window to set itself > up and for the initial browser to be ready to do things with it. > > If you care - here’s why: without them, you run the risk of falling into > the mistake I was seeing in a number of tests, which was opening a new > window, waiting for its load event, telling the initial browser to load > some URI, and then waiting for BrowserTestUtils.browserLoaded to resolve > before proceeding. This technique will fail frequently, because sometimes > the message to load the URI will arrive at content after it has loaded its > initial about:blank, but before the parent has had an opportunity to hear > that the initial about:blank has been loaded. This means that the > browserLoaded Promise will sometimes resolve even though the URI you > requested hasn’t finished (or maybe even started) loading. > > Anyhow, with the patches in the bug, I appear to have fixed all of the > tests that fall into this trap - but if you write a new test that’s opening > a window that’s failing frequently and you don’t know why, it might be > because you need to use those BrowserTestUtils methods. > > Feel free to follow up with me in this thread or over IRC if there are any > questions or concerns regarding bug 1261842. > > Thanks, > > -Mike > ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
PSA - With e10s enabled, initial browser in new windows will soon be remote by default
If you don’t write tests that open browser windows or work on the front-end of Firefox, you can probably ignore this. The initial browser in browser.xul is non-remote by default. This means that as soon as it browses to some URL that can be loaded in the content process (about:home, for example), we kick off the remoteness flipping machinery that swaps out that initial browser and puts a remote one down in its place to load the content in the child process. In bug 1261842, I’m working on switching things around so that the initial browser is remote as soon as possible (there’s still some set-up required in JS, so I can’t just add the remote attribute to that initial browser). Doing the flip sooner means we don’t have to set up the content viewer for the non-remote browser (which gets immediately destroyed after the remoteness flip), which is good for window-opening performance. Normally, I wouldn’t bother these two mailing lists with this kind of information, but while working on these patches, I’ve had to fix a bunch of tests that worked properly under the old assumption (initial browser is non-remote) but not under the new one (initial browser is remote), and I wanted to give a PSA on those writing tests that open windows. Here’s the deal: If you’re writing browser mochitests and you need to open a window, please use BrowserTestUtils.openNewBrowserWindow. If you need to trigger window openings via another mechanism (like window.open in content), but need to do things with the newly opened window, set up a Promise with BrowserTestUtils.waitForNewWindow. Both of these methods are doing the right thing in terms of waiting for the new window to set itself up and for the initial browser to be ready to do things with it. If you care - here’s why: without them, you run the risk of falling into the mistake I was seeing in a number of tests, which was opening a new window, waiting for its load event, telling the initial browser to load some URI, and then waiting for BrowserTestUtils.browserLoaded to resolve before proceeding. This technique will fail frequently, because sometimes the message to load the URI will arrive at content after it has loaded its initial about:blank, but before the parent has had an opportunity to hear that the initial about:blank has been loaded. This means that the browserLoaded Promise will sometimes resolve even though the URI you requested hasn’t finished (or maybe even started) loading. Anyhow, with the patches in the bug, I appear to have fixed all of the tests that fall into this trap - but if you write a new test that’s opening a window that’s failing frequently and you don’t know why, it might be because you need to use those BrowserTestUtils methods. Feel free to follow up with me in this thread or over IRC if there are any questions or concerns regarding bug 1261842. Thanks, -Mike ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
