1437538 - New tab console logs do not persist

Status: RESOLVED FIXED

(DevTools :: Console, defect, P3)

Description

[Oriol Brufau [:Oriol]]

1. Open a new tab
2. Open the web console
3. Make sure "Persist Logs" is enabled
4. Enter 123
5. In the location bar, write example.com, press enter

Result: the console is cleared.

Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=dd4bf41ca47d3e3036b194a61af4983c58dd14b2&tochange=1bc06a4d147320b0976467a9771ec4411dbe66d1

This happens even if e10s is disabled.

Comment 1

[Nicolas Chevobbe [:nchevobbe][PTO - back on October 21]]

Attachment: Feb-12-2018 16-27-27.gif

Oh yes, looks like the entire toolbox is reloaded (See attachment), but only the first time. If you go back to the new tab, type something and navigate again, your logs do persist.

Thanks Oriol for the regression window, this is very helpful.

Mike, for some reason I can't ni? the author of the patch, and since you review the patch, would you know what could explain this ?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

(In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)
> Mike, for some reason I can't ni? the author of the patch, and since you
> review the patch, would you know what could explain this ?

Yeah, we do a process transition when browsing away from the about:newtab page in some cases (namely, when there are few pre-existing tabs). So this is a bit similar to the issue we had with DevTools when switching away from the old about:newtab to a new page - a process flip would cause the toolbox to close.

I'm uncertain if DevTools has a way of dealing with process transitions like this. Maybe bgrinstead knows?

Comment 3

[Brian Grinstead [:bgrins]]

(In reply to Mike Conley (:mconley) (:⚙️) (Totally backlogged on reviews and needinfos) from comment #2)
> (In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)
> > Mike, for some reason I can't ni? the author of the patch, and since you
> > review the patch, would you know what could explain this ?
> 
> Yeah, we do a process transition when browsing away from the about:newtab
> page in some cases (namely, when there are few pre-existing tabs). So this
> is a bit similar to the issue we had with DevTools when switching away from
> the old about:newtab to a new page - a process flip would cause the toolbox
> to close.
> 
> I'm uncertain if DevTools has a way of dealing with process transitions like
> this. Maybe bgrinstead knows?

Going to forward this question to Alex who worked on the process transition. I suspect it won't be easy to restore the console messages in this case, and if it's not I don't think it'd be worth the effort just to handle persisting logs from the new tab.

Mike: by the way, are you aware of any plans to migrate the new tab page to a content process?

Comment 4

[Oriol Brufau [:Oriol]]

(In reply to Mike Conley (:mconley) (:⚙️) (Totally backlogged on reviews and needinfos) from comment #2)
> (In reply to Nicolas Chevobbe [:nchevobbe] from comment #1)
> Yeah, we do a process transition

I disable e10s to avoid this kind of annoyances. I only have 1 process, so there can't be any process transition. So why do I see this? Something is wrong.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

(In reply to Brian Grinstead [:bgrins] from comment #3)
> Mike: by the way, are you aware of any plans to migrate the new tab page to
> a content process?

There's the rub - the newtab page _is_ running in the content process.

The problem that bug 1376895 (the one that caused this regression) meant to solve is actually because of this.

We have something called the about:newtab preloader, which preloads the next about:newtab after you've opened a tab. That way, the next tab is ready to roll. After we moved about:newtab to the content process with Activity Stream, that meant that if you opened a new tab to give yourself a total of 2 tabs, the next about:newtab would preload in the background and allocate another content process.

For AWSY, the potentially unused content process used by the preloaded about:newtab was no bueno. Bug 1376895 worked around this by having the preloaded browser choose a pre-existing content process to preload into instead of creating a new process. However, once the page browses _away_ from about:newtab, if we haven't hit the limit on content processes, it'll spawn a new content process and swap out to it so that we can still use all content processes that we're set up to.

I hope the above explanation makes sense.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

(In reply to Oriol Brufau [:Oriol] from comment #4)
> I disable e10s to avoid this kind of annoyances. I only have 1 process, so
> there can't be any process transition. So why do I see this? Something is
> wrong.

How are you disabling e10s?

Comment 7

[Oriol Brufau [:Oriol]]

(In reply to Mike Conley (:mconley) (:⚙️) (Totally backlogged on reviews and needinfos) from comment #6)
> How are you disabling e10s?

The checkbox in preferences was removed but I can still set browser.tabs.remote.autostart=false in about:config

Comment 8

[Alexandre Poirot [:ochameau]]

Yes, process transition just reopens a toolbox. Unfortunately, there is nothing easy we can do at framework/toolbox level.
Hopefully the work to address site isolation will help making a toolbox be able to seemlessly target multiple processes at the same time. Right now everything is designed around "target" object, which relates to one precise TabActor instance. Moving away from this pattern is going to require significant effort across our codebase.

Comment 9

[Oriol Brufau [:Oriol]]

(In reply to Mike Conley (:mconley) (:⚙️) (Totally backlogged on reviews and needinfos) from comment #5)
> However, once the page browses _away_
> from about:newtab, if we haven't hit the limit on content processes, it'll
> spawn a new content process and swap out to it so that we can still use all
> content processes that we're set up to.

With e10s, I get the problem even after reaching the limit on content processes. about:support says I'm using 5 processes out of the 4 available.

So I repeat that something is wrong, this is not only caused by a process transition.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

(In reply to Oriol Brufau [:Oriol] from comment #9)
> So I repeat that something is wrong, this is not only caused by a process
> transition.

I'm not sure I follow. Even if you are using all content processes, I believe we still do a process transition after we choose the appropriate process to browse you to the site you selected. So having all content processes used does not mean that a process transition will not occur. And a process transition causes the behaviour you're experiencing.

Or am I missing something?

Comment 11

[Oriol Brufau [:Oriol]]

Please correct if I'm wrong, but:

1. I open a new tab, and enter something into its web console.
2. I open the Browser Content Toolbox and run Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime).processID
   The PID is 7968
3. I click some link, the console reloads.
4. I close the Browser Content Toolbox, open it again, and get the PID.

After some tries, at step 4 I get the same 7968 PID. This means there was no process transition, but I still got the problem.

Comment 12

[Oriol Brufau [:Oriol]]

OK, so what happens is that newFrameloader is always set to true
https://searchfox.org/mozilla-central/rev/88e89261ea81860f44a44ed068cf5839aea5def6/browser/base/content/browser.js#1058

Therefore updateBrowserRemoteness does not abort even if nothing changes
https://searchfox.org/mozilla-central/rev/88e89261ea81860f44a44ed068cf5839aea5def6/browser/base/content/tabbrowser.xml#1945-1950

mustChangeProcess is always set to true too, even if e10s is disabled!

Comment 13

[Patrick Brosset <:pbro>]

Adjusting the priority and tracking flags based on comment 3.

Comment 14

[Nicolas Chevobbe [:nchevobbe][PTO - back on October 21]]

It is working if you set devtools.target-switching.enabled to true.

Comment 15

[Alexandre Poirot [:ochameau]]

devtools.target-switching.enabled is now enabled by default and this should work.