1566215 - Consider switching the layout debugger to use e10s

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[:Gijs (he/him)]

Right now the layout debugger loads resources in a non-e10s <browser>.

It'd be safer, but also probably less convenient in some ways to have it load things in a remote browser instead. bug 1565965 comment #3 suggests we would ideally switch to using e10s. OTOH, based on https://groups.google.com/d/msg/mozilla.dev.platform/cJMzxi7_PmI/NfM6D3OPBQAJ , it seems some folks prefer having something single-processed to help with debugging.

I don't know to what degree switching to a single-child-process (so e10s-enabled) layout debugger would hamper debugging - I suppose it's somewhat less annoying than "real" Firefox but probably more annoying than the current non-e10s state.

I don't really have a horse in this race, but it seemed worth getting a bug on file seeing as I couldn't find any.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think having e10s is more convenient when debugging in rr, because then all the stuff you care about is in its own process!

Comment 2

[Cameron McCormack (:heycam)]

Adding remote="true" to the <browser> element is enough to make it use a content process, but all of the nsLayoutDebuggingTools methods that need to poke at the docshell, e.g. for frame tree dumping, need another mechanism to ask the content process to do the work.

Comment 3

[Cameron McCormack (:heycam)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=fe192d46fe3f587f085a315218348ab2ba36e1f3

Comment 4

[Cameron McCormack (:heycam)]

Attachment: Bug 1566215 - Part 1: Remove some unused nsLayoutDebuggingTools code.

Comment 5

[Cameron McCormack (:heycam)]

Attachment: Bug 1566215 - Part 2: Merge nsILayoutDebugger functionality into nsLayoutDebuggingTools.

Comment 6

[Cameron McCormack (:heycam)]

Attachment: Bug 1566215 - Part 3: Move debugging state management into layoutdebug.js.

Many of these features are non-functional these days, but I'll keep them
hooked up in case we decide to fix them.

Comment 7

[Cameron McCormack (:heycam)]

Attachment: Bug 1566215 - Part 4: Use message manager to send layout debugger commands to content.

Comment 8

[Cameron McCormack (:heycam)]

Attachment: Bug 1566215 - Part 5: Use a remote browser in the layout debugger.

Comment 9

[Cameron McCormack (:heycam)]

Attachment: Bug 1566215 - Part 6: Undo the insecure URI loading exception for the Layout Debugger.

Comment 10

[Cameron McCormack (:heycam)]

One thing I only just noticed after uploading the patches is that I can no longer view file: URLs with these changes, unless I turn off sandboxing. I think that the file: URL is being loaded in a regular Web content process, and not a file: content process. Also when I load say about:config, that also gets loaded in the content process.

Gijs, do you know what I might need to do beyond using remote="true" on the <browser> to cause URLs to be loaded in the expected process?

Comment 11

[Cameron McCormack (:heycam)]

I think maybe I need to do some things similar to _loadURI in browser.js.

Comment 12

[Cameron McCormack (:heycam)]

I've worked it out.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/32f959e898c6
Part 1: Remove some unused nsLayoutDebuggingTools code. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/184025bba151
Part 2: Merge nsILayoutDebugger functionality into nsLayoutDebuggingTools. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/f84c119572a1
Part 3: Move debugging state management into layoutdebug.js. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/ffbc0f9c06c4
Part 4: Use message manager to send layout debugger commands to content. r=dbaron
https://hg.mozilla.org/integration/autoland/rev/5d1cf5706da5
Part 5: Use a remote browser in the layout debugger. r=dbaron,Gijs
https://hg.mozilla.org/integration/autoland/rev/5b7f4799a1a8
Part 6: Undo the insecure URI loading exception for the Layout Debugger. r=dbaron

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/db8f3ee41bdf
Fix ESlint failure on a CLOSED TREE

Comment 15

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/32f959e898c6
https://hg.mozilla.org/mozilla-central/rev/184025bba151
https://hg.mozilla.org/mozilla-central/rev/f84c119572a1
https://hg.mozilla.org/mozilla-central/rev/ffbc0f9c06c4
https://hg.mozilla.org/mozilla-central/rev/5d1cf5706da5
https://hg.mozilla.org/mozilla-central/rev/5b7f4799a1a8
https://hg.mozilla.org/mozilla-central/rev/db8f3ee41bdf