Closed Bug 1287004 Opened 4 years ago Closed 3 years ago
Support remote <browser> elements in windowless browsers
This will be one of the last things we do for bug 1190679. I tried this a pretty big problem. Currently we create a <browser> element inside a windowless docshell . The windowless docshell uses a PuppetWidget. The layers code doesn't seem to be able to cope with hosting remote content inside of a PuppetWidget. In the parent process, we crash at  because GetRemoteRenderer() returns null. That makes sense since we don't have a CompositorChild for the windowless docshell. In the child process, we assert at . I don't really understand that part. Matt, can you offer any advice here? Should we try to create a CompositorChild for the windowless docshell, or should we try to avoid accessing that object. We're not going to ever draw these things, but we do need to support things like canvas getImageData (but only in the remote browser element).  http://searchfox.org/mozilla-central/rev/e269c68bc594b4a858624d73f0d9eb002f3c0844/xpfe/appshell/nsAppShellService.cpp#494  http://searchfox.org/mozilla-central/rev/e269c68bc594b4a858624d73f0d9eb002f3c0844/layout/ipc/RenderFrameParent.cpp#123  http://searchfox.org/mozilla-central/rev/e269c68bc594b4a858624d73f0d9eb002f3c0844/gfx/layers/ipc/ShadowLayers.cpp#243
TabChild::InitRenderingState is supposed to be called when we setup the tab in the child document, and that provides the shadow manager (PLayerTransactionChild) to use for forwarding layers to the parent. If we don't want to ever render, we shouldn't need to do this. The call site at  seems like it is happening from rendering though, can we stop this from happening?
Summary: Use remove <browser> element for background page → Use remote <browser> element for background page
This was the easiest way I could think of doing this. It seems to work okay.
Attachment #8788263 - Flags: review?(matt.woodrow)
Attachment #8788263 - Flags: review?(matt.woodrow) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/663cde4dd044 Give PuppetWidgets in parent process a compositor bridge (r=mattwoodrow)
Or if you like Android crashes, as nobody does, https://treeherder.mozilla.org/logviewer.html#?job_id=35798400&repo=mozilla-inbound Or if you like Windows assertions and then crashes, https://treeherder.mozilla.org/logviewer.html#?job_id=35800549&repo=mozilla-inbound
Sorry, this was green a few days ago.
Assignee: nobody → wmccloskey
Component: WebExtensions: Untriaged → WebExtensions: General
Priority: -- → P2
3 years ago
Duplicate of this bug: 1280028
It looks like other work is also blocking this (bug 1280028). David said he would work on it soon.
It looks like the ClientLayerManager approach would break because RenderFrame assumes it'll have a CompositorBridgeChild singleton. In addition we'd have no CompositorWidget, and we would probably get into trouble anyway without an HWND (at least, for d3d11). Is there any reason we can't just use a BasicLayerManager instead? It seems to pass all the extension tests, and try looks good as well.
This version does not use a retained BasicLayerManager.
FWIW, the previous patch fixed the crashes I was seeing in bug 1280028 (haven't checked the new one, but I presume it'll work, too). I wonder if it's worth a crash test of sorts? :-)
Attachment #8835258 - Flags: review?(matt.woodrow) → review+
Testing with this more, my stdout/stderr is spammed with lots of messages like this: |[C90][GFX1-]: ClientLayerManager::BeginTransaction with IPC channel down. GPU process may have died. (t=36.4959) [GFX1-]: ClientLayerManager::BeginTransaction with IPC channel down. GPU process may have died. Could be coincidental, but seems related. Doesn't happen on today's nightly run with the same profile (but without this change and my patch for bug 1338215).
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/61e17a4c8397 Give browser-process PuppetWidgets a BasicLayerManager. (bug 1287004, r=mattwoodrow)
Summary: Use remote <browser> element for background page → Support remote <browser> elements in windowless browsers
3 years ago
Depends on: 1339607
You need to log in before you can comment on or make changes to this bug.