Open Bug 1568827 Opened 6 years ago Updated 6 months ago

[meta] Make highlighters able to highlight elements even in oop iframes

Categories

(DevTools :: Inspector, enhancement, P3)

enhancement

Tracking

(Fission Milestone:Future)

Fission Milestone Future

People

(Reporter: pbro, Unassigned)

References

(Depends on 6 open bugs, Blocks 1 open bug)

Details

(Keywords: meta, Whiteboard: dt-fission-future)

Attachments

(1 obsolete file)

With Fission, all highlighters must continue correctly highlighting any nodes, even in oop iframe.
Highlighters that draw themselves all the way to the edge of the top-browser window should continue to do so.
Nodeinfobars should continue to be fully visible, whatever the position and size of the highlighted node.

The main problem to solve here is that all of the DevTools highlighters get references to DOM nodes that may live in any frame, including out-of-process frames.
Highlighters need direct access to those DOM nodes in order to find their coordinates, measure them and respond to changes (if a node's position or size is animated for example).

Some form of server-side cross-process communication seems required here. Indeed the DevTools client only uses a very simple highlighter API (show/hide) and lets the server do the heavy lifting.

Solution 1: Make the client do more work

We could split the highlighter code in 2 parts: the first part would be responsible for accessing to the DOM node, finding its coordinates, etc. and then serializing this information. The second part would actually render the highlighter using this data.
This would be done in 2 requests: a first request to to get the data about the node (request sent to an actor living in the same process as the node), and then upon receiving the response, a second request to the highlighter to draw.
The highlighter actor would live in the top-level content process, so it can keep on drawing over the entire page.

This would probably be really bad for performance, especially when debugging remote devices.
Most highlighters today try to "follow" the node when it moves around. This solution would introduce a noticeable lag.

Solution 2: Make the server do this work

Like today, we would send the request to draw the highlighter to the top-level content process.
The highlighter actor would then cross process boundaries by using whatever Fission message passing API exists to get the data required to draw the node from whatever content process it lives in.

This would probably be better in terms of performance, and has the advantage of living the client-side code untouched.
However this is probably a no-go in terms of security (content processes talking to other content processes).

Solution 3: escaping the clipping that happens at the graphics level

To be investigated, I'm not sure there is a way to do this.
Here we would always draw the highlighters in the process where the dom node they are highlighting live. So if we're trying to highlight a node that lives in an oop iframe, then we'd send the request to that iframe only.
And then we'd work with the layout and graphics engine people to find out whether it would be possible to not clip our special highlighter content but draw it on the entire content area.
Unlikely to be feasible, but I'm listing this for the sake of completeness.

This would be ideal for DevTools because we wouldn't change either the client or server code much. The only part that would change is the drawing offset code we add today to account for iframes and such.

Solution 4: Drawing in the parent process instead

In a way, this is going back to how highlighters used to work maybe 5 years ago.
Our highlighters code would live in the parent process (not the content process) and would draw there. That means finding a place in the browser UI markup to insert the DOM used to draw.
DevTools would send a request to whatever content process the DOM node lives in, and that process would send an IPC message to the parent process (communication between content process and parent process is totally OK). That message would contain enough information about the node for a highlighter to draw itself.
More messages would be sent in case of animations, reflows and what not.
The highlighter would just wait for these messages and draw itself in the parent process.

Performance should likely not be too much of an issue here.

The main changes are:

Highlighters are split into 2 parts
Part 1 measures the nodes (or gets any data about the node that requires a reference to the DOM node itself). This happens in the content process where the DOM node lives. This is handled by the traditional DevTools-style actor.
Part 1 also listens for any events that might cause the node to change shape or position and require the highlighter to be re-drawn.
Part 2 draws the highlighter (given the node measurements retrieved by part 1). This is handled by a new JSWindowActor-style actor (probably to be defined here).

The part 2 of the highlighters now lives in the parent process, always, and the markup required by the highlighters to draw themselves are injected in the browser.xhtml tree (not in the nsCanvasFrame anonymous content container anymore, which would actually be a really good thing for developer ergonomics).
Communication happens with JSWindowActors like the rest of the browser features.
This means the client -> server protocol communication requires only 1 request like today: the initial one from, say, the inspector panel to highlight a node. It is important to keep it this way since this transport layer may be slow (think USB-based remote debugging of GeckoView).
This also means we need a way, from the DevTools highlighter actor, to connect to a JSWindowActorChild (the question marks in the diagram).

Next explorations:
How will this work on GeckoView? GeckoView has a similar process structure as desktop. Learnings from talking to snorp:
it sounds like this should Just Work in GeckoView. We have a sort of browser.xul right now in GeckoView (named geckoview.xul) and it is basically just <window><browser/></window> with some JS. It can probably be converted to html fairly easily.
Only difference from desktop is there is no tabbed browser. There are as many windows as there are tabs. So DevTools would have to consider each <browser> element.
Can we include our insane highlighters markup in the browser UI? Will talk to mconley about this.
How will this work for the browser toolbox? There is no grand-parent process. But we could special-case this a bit and just use the same highlighter.

Pros & Cons:

  • No more nsCanvasFrame API
  • Probably the least perf impact of all solutions
  • Should make it easier to use the same highlighters for the browser UI itself
  • Special case for GeckoView apps
Depends on: 1570602
Priority: P3 → P2
Whiteboard: dt-fission
Depends on: 1572651
Depends on: 1572652
Depends on: 1572653
Depends on: 1572654
Depends on: 1572655
Depends on: 1572657
Depends on: 1572658
Depends on: 1572659
Depends on: 1572661
Depends on: 1572662
Depends on: 1572663
Priority: P2 → P3
Whiteboard: dt-fission → dt-fission-reserve

Tentatively moving all bugs whose summaries mention "Fission" (or other Fission-related keywords) but are not assigned to a Fission Milestone to the "?" triage milestone.

This will generate a lot of bugmail, so you can filter your bugmail for the following UUID and delete them en masse:

0ee3c76a-bc79-4eb2-8d12-05dc0b68e732

Fission Milestone: --- → ?
Depends on: 1598307
Attachment #9081962 - Attachment is obsolete: true
Fission Milestone: ? → ---

Tracking for Fission Nightly (M6)

Fission Milestone: --- → M6
Keywords: meta
Summary: Make highlighters able to highlight elements even in oop iframes → [meta] Make highlighters able to highlight elements even in oop iframes
Depends on: 1592978
Depends on: 1608784
Depends on: 1618266
Depends on: 1618245
Depends on: 1601346
Depends on: 1607756
Depends on: 1623667
No longer blocks: 1618267
Depends on: 1618267

dt-fission-reserve bugs do not need to block Fission Nightly (M6).

Fission Milestone: M6 → ---
Depends on: 1646028
No longer depends on: 1646028
No longer depends on: 1601346

Tracking dt-fission-reserve bugs for Fission MVP until we know whether they really need to block Fission or not.

Fission Milestone: --- → MVP

Moving old "dt-fission-reserve" bugs to "dt-fission-future" because they don't block Fission MVP.

Fission Milestone: MVP → Future
Whiteboard: dt-fission-reserve → dt-fission-future
Severity: normal → S3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: