Closed
Bug 846421
Opened 12 years ago
Closed 1 year ago
mozbrowser screenshot API doesn't capture nested remote mozbrower content
Categories
(Core :: Graphics, defect)
Tracking
()
People
(Reporter: cjones, Unassigned)
References
Details
(Whiteboard: permafail [nested-oop])
Attachments
(1 file)
(deleted),
patch
|
Details | Diff | Splinter Review |
I thought for sure this was on file somewhere else, but can't find it. Symptom 1. Open b2g browser app, load some page 2. Long-press HOME to open task switcher Notice that the content area in the browser display is white. The problem is that the screenshot API does a drawWindow() in process. mozbrowser embedders can't build a layer tree for remote content, so we instead just don't paint those regions. (So they show up as white, from the default background.) We have two ways to fix this A. Make mozbrowser responsible for gathering sceenshots of nested mozbrowser content. Hacking something together that worked 80% could sorta work, but it would make a slow process even slower, and wouldn't handle lots of cases like transforms and async pan-zoom. B. Switch the screenshot API to requesting a capture from the compositor, for the subtree rooted at the window requesting capture. This would be WAAAY faster, and would handle the edge cases above, but ... it also wouldn't work when the frame was hidden, since there wouldn't be a layer tree for it or its descendents. Since we're stuck between a rock and a hard place, the best solution may be a mix. Option (A) is probably too fragile, but we could use the current impl when frames are hidden and suffer the unpainted regions, but switch to option (B) when they're not.
Comment 2•10 years ago
|
||
is this planned to be fixed? This impact URL sharing over NFC, which is 2.0 key feature
Blocks: b2g-NFC-2.0
Flags: needinfo?(fabrice)
Updated•10 years ago
|
feature-b2g: --- → 2.0
Comment 3•10 years ago
|
||
(In reply to Wesley Huang [:wesley_huang] from comment #2) > is this planned to be fixed? > This impact URL sharing over NFC, which is 2.0 key feature No one is assigned, and from comment 0 it looks very non trivial. Kanru, do you have time to look at this?
Updated•10 years ago
|
Flags: needinfo?(fabrice)
Comment 5•10 years ago
|
||
May I know why URL sharing over NFC needs to be able to take screenshots of embedded remote frame? I think I don't have time to look at this before 2.0 FC..
Flags: needinfo?(kchen)
Comment 6•10 years ago
|
||
This is for bug 998187. let me know if you have further question. Anyone else can look into this?
Flags: needinfo?(kchen)
Comment 7•10 years ago
|
||
Just an idea, I think if we could make CanvasRenderingContext2D::DrawWindow accepts (Window or HTMLIFrameElement) then we could reuse the canvas drawing API to draw remote iframe from parent process directly.
Flags: needinfo?(kchen)
Comment 8•10 years ago
|
||
Hi CJ, Ivan, This seems like a graphic issue.
Flags: needinfo?(itsay)
Flags: needinfo?(cku)
Comment 9•10 years ago
|
||
remove feature-b2g and nominate 2.0? This is an obvious defect to new feature (URL sharing thru NFC)
blocking-b2g: --- → 2.0?
feature-b2g: 2.0 → ---
Comment 10•10 years ago
|
||
Talk to Peter and he will take a look this one from graphic side and see how this one can be proceeded.
Flags: needinfo?(itsay) → needinfo?(pchang)
Comment 12•10 years ago
|
||
(In reply to Kan-Ru Chen [:kanru] from comment #7) > Just an idea, I think if we could make CanvasRenderingContext2D::DrawWindow > accepts (Window or HTMLIFrameElement) then we could reuse the canvas drawing > API to draw remote iframe from parent process directly. To support the nested content screen capture, there is one option to use compositor to capture the screenshot, like the fullscreen catpure(home+power key). But this also needs to filter unnecessary components(status bar) to present the app screenshot.
Flags: needinfo?(pchang)
Updated•10 years ago
|
Assignee: nobody → pchang
Comment 13•10 years ago
|
||
Plus this bug in 2.0 to fix the vendor dependent hard code in pixel format.
blocking-b2g: 2.0? → 2.0+
Updated•10 years ago
|
status-b2g-v2.0:
--- → affected
Whiteboard: [2.0-flame-test-run-1]
Comment 15•10 years ago
|
||
We can flag it however we want to, if we're using flags to indicate priority, but there is nothing about this that's a bug fix. We're talking about adding a new API and functionality, surely it's a feature.
Comment 16•10 years ago
|
||
Now I have a local WIP to use compositor to capture screenshot for browser app and it works with NFC UI (show the browser screenshot). In gecko, it requires the chrome privilege to capture screenshot by compositor because of security concern. Now I'm checking the security policy in compositor flow.
Comment 17•10 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #16) > Now I have a local WIP to use compositor to capture screenshot for browser > app and it works with NFC UI (show the browser screenshot). > > In gecko, it requires the chrome privilege to capture screenshot by > compositor because of security concern. Now I'm checking the security policy > in compositor flow. With above patch, I could capture correct browser app screenshot by compositor(without status bar) but also generate one frame without status bar displayed on the screen. This reason is gecko calls compositor to composite again and then read pixel back to save as screenshot. In my patch, I hack the root frame checking[1] for browser(chrome) because browser always fails to pass this checking. After checking with frame tree, the following aFrame is browser's viewport frame. The following rootFrame is the viewport frame of status bar. Force using widget layer by skipping this checking will cause the frame without status bar composed. 06-04 04:41:19.912 1187 1187 I Gecko : pchang PresShell::RenderDocument checking aFrame 0xb31052b8 RootFrame 0xad07a2b8 [1]http://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#4795
Comment 18•10 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #17) > (In reply to peter chang[:pchang][:peter] from comment #16) > > Now I have a local WIP to use compositor to capture screenshot for browser > > app and it works with NFC UI (show the browser screenshot). > > > > In gecko, it requires the chrome privilege to capture screenshot by > > compositor because of security concern. Now I'm checking the security policy > > in compositor flow. > > With above patch, I could capture correct browser app screenshot by > compositor(without status bar) but also generate one frame without status > bar displayed on the screen. This reason is gecko calls compositor to > composite again and then read pixel back to save as screenshot. > Since I try to drawWindow with non-top root frame, it generates the new layout changes and layers tree changes in compositor side. But it supposes no need to compose the new changes to screen when generate snapshot. But there is a timing issue between these layout changes called and snapshot called. That's why I see one frame without status bar displayed. There is one idea to add END_NO_COMPOSITE for EndTransaction to prevent the composition to screen when the layout changes, and only composite to snapshot's render target. > In my patch, I hack the root frame checking[1] for browser(chrome) because > browser always fails to pass this checking. After checking with frame tree, > the following aFrame is browser's viewport frame. The following rootFrame is > the viewport frame of status bar. > > Force using widget layer by skipping this checking will cause the frame > without status bar composed. > > 06-04 04:41:19.912 1187 1187 I Gecko : pchang PresShell::RenderDocument > checking aFrame 0xb31052b8 RootFrame 0xad07a2b8 > > [1]http://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell. > cpp#4795
Comment 19•10 years ago
|
||
This is a long-standing issue that's non-trivial to implement, so I don't think this should block nor be considered a 2.0 feature. As for the NFC concern, I think we should adjust the UX to take into account the fact that screenshots don't work in the browser.
blocking-b2g: 2.0+ → 2.0?
Comment 20•10 years ago
|
||
By drawing the non-top root frame, it causes the layout and retained layers changed. I try to prevent the changed retained layers composition on the screen but the composition still triggers when there is new frame update from other process. Therefore, the way to force drawing the non-top root frame by using widget layer causes several side effects. There is another option to query the existed layer based the input 'frame' and then ask compositor to compose the sub layer tree as snapshot.
Comment 21•10 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #19) > This is a long-standing issue that's non-trivial to implement, so I don't > think this should block nor be considered a 2.0 feature. As for the NFC > concern, I think we should adjust the UX to take into account the fact that > screenshots don't work in the browser. wesley, I'm able to capture the browser screenshot but has other side effects in comment 20. Any comment to fix this problem from UX side?
Flags: needinfo?(whuang)
Comment 22•10 years ago
|
||
ni? Juwei as she takes charge of NFC UX. Let's talk face to face.
Flags: needinfo?(whuang) → needinfo?(jhuang)
Comment 23•10 years ago
|
||
Per discussion with Peter and Juwei, let's leave with it in 2.0 considering risk of fixing at this moment. nominate 2.1? so that we'll triage when 2.1 begins.
blocking-b2g: 2.0? → 2.1?
Updated•10 years ago
|
Flags: needinfo?(jhuang)
Updated•10 years ago
|
No longer blocks: b2g-NFC-2.0
Updated•10 years ago
|
Whiteboard: [2.0-flame-test-run-1] → [2.0-flame-test-run-1][nested-oop]
Updated•10 years ago
|
QA Whiteboard: [COM=NFC]
Updated•10 years ago
|
Assignee: pchang → chung
Comment 25•10 years ago
|
||
The priority of this item has been lowered and moved to backlog of graphic work for Firefox OS. Adjust the flags accordingly.
Updated•10 years ago
|
Whiteboard: [2.0-flame-test-run-1][nested-oop] → [2.0-flame-test-run-1] [2.0-flame-test-run-2] [nested-oop]
Updated•10 years ago
|
Whiteboard: [2.0-flame-test-run-1] [2.0-flame-test-run-2] [nested-oop] → permafail [nested-oop]
Comment 26•10 years ago
|
||
A working WIP. The implementation now tries to preserve the semantics of DrawWindow flags, and it will always repaint all the layers since retaining fail. it needs major clean up of debug logs, and some further investigation: (1) WHY Layer Retaining seems fail (2) What about security concerns (3) Preventing display while snapshot (if needed) (4) Compose into SurfaceDescriptor directly rather than readback from framebuffer.
Updated•10 years ago
|
QA Whiteboard: [COM=NFC]
Assignee | ||
Updated•10 years ago
|
blocking-b2g: backlog → ---
Updated•9 years ago
|
Blocks: TV_FxOS2.5
Updated•9 years ago
|
No longer blocks: TV_FxOS2.5
Comment 29•2 years ago
|
||
The bug assignee didn't login in Bugzilla in the last 7 months, so the assignee is being reset.
Assignee: ffantasy1999 → nobody
Updated•2 years ago
|
Severity: normal → S3
Comment 30•1 year ago
|
||
drawWindow was replaced with a better async method for fission.
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → INVALID
You need to log in
before you can comment on or make changes to this bug.
Description
•