Closed Bug 783066 Opened 13 years ago Closed 13 years ago

Gaia homescreen background isn't transparent when it's loaded out of process

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla18
Project Flags:
blocking-kilimanjaro +
blocking-basecamp +

People

(Reporter: cjones, Assigned: cjones)

References

Details

Attachments

(1 file, 3 obsolete files)

Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments, v2
7.69 KB, patch
roc
: review+
Details | Diff | Splinter Review
Reproduces on desktop as well. Might be a platform bug, but let's start off in b2g component.
blocking-basecamp: --- → +
Vivien fixed or worked around this somehow in gaia, so not a blocker.
blocking-basecamp: + → ---
False alarm, still a problem.
blocking-basecamp: --- → +
Vivien did you file a gaia issue for drawing the background in the homescreen itself?
Status: NEW → RESOLVED
blocking-basecamp: + → ---
Closed: 13 years ago
Resolution: --- → INVALID
Epic bz fail!
Status: RESOLVED → REOPENED
blocking-basecamp: --- → +
Resolution: INVALID → ---
Vivien, do you have a gaia issue open for loading the background in the homescreen app itself yet?
No longer a blocker since we have a better gaia fix.
blocking-basecamp: + → -
blocking-kilimanjaro: --- → +
Summary: Background of gaia homescreen doesn't display when it's loaded out of process → Gaia homescreen background isn't transparent when it's loaded out of process
Etienne points out that this bug affects the dialer attention screen (and I guess all the other ones), for which there isn't a workaround.
blocking-basecamp: - → +
Component: General → Layout
Product: Boot2Gecko → Core
The problem here seems to be that out-of-process content is picking up a default opaque white background from somewhere (CanvasFrame? I dunno). However, in my simple tests, subdocument frames don't get a default background, they seem to be transparent by default. Remote content is always a subdocument frame, so I think we should switch this to match same-process behavior. I poked at nsCanvasFrame and nsViewportFrame but couldn't make them stop drawing a background. roc/tn/mats, where should I be looking here?
Ah! Thanks for the pointer.
I see us getting to the ColorLayer optimization in FrameLayerBuilder for the nsDisplayCanvasBackground. Its "mExtraBackgroundColor" is set to rgba(1,1,1,1).
mExtraBackgroundColor comes from only one place http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#4835 which is called from the function right below it.
If I hack that code out, the white background goes away, but the root ContainerLayer still thinks it's opaque and I get a black background.
The layer probably is still getting the opaque flag set? Depending on which code you commented out this is either because the code still thinks the layer is opaque but we just don't draw opaque or because we are setting the opaque flag from somewhere else.
Attached patch Hackery (obsolete) — — Splinter Review
Yeah, I mid-aired you, my comment was ambiguous in context. Here's the hackery doo that gets us to no-white-background, but the root ContainerLayer still thinks it's opaque so we get a black background (unpainted CONTENT_COLOR surface, I would guess).
Ok that makes sense. Then I think we want to figure out where the opaque mExtraBackgroundColor is coming from and change that code to not do that for (only) the situation that is important here.
My quick look best guess is it is coming from PresShell::ComputeBackstopColor maybe?
Forcing that to return rgba(0,0,0,0) in content processes doesn't change anything: opaque white background, ColorLayer optimization kicks in, and the root ContainerLayer thinks it's opaque content.
Attached patch Hackery v2 (obsolete) — — Splinter Review
This hackery doo gets rid of the background.
Attachment #658233 - Attachment is obsolete: true
Making PuppetWidget pretend to be transparent doesn't change anything, but I guess that's expected.
I don't know how to write a reftest for this in the current incarnation of the reftest harness. We would need to allow reftests to set a non-default background color for the xul:window in the master process.
Assignee: nobody → jones.chris.g
Attachment #658268 - Attachment is obsolete: true
Attachment #658310 - Flags: review?(roc)
Comment on attachment 658310 [details] [diff] [review] Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments Review of attachment 658310 [details] [diff] [review]: ----------------------------------------------------------------- ::: layout/base/nsPresContext.cpp @@ +2343,5 @@ > +bool > +nsPresContext::IsCrossProcessRootContentDocument() > +{ > + return (XRE_GetProcessType() == GeckoProcessType_Default && > + IsRootContentDocument()); Can you explain why this is correct? If we are a GeckoProcessType_Content, shouldn't this be able to return true in at least some cases?
Oh, I misunderstood what "content" referred to in this code. I see now that it means "not chrome". You're right, we need to make this check more precise. Will put up a v2 patch in a minute.
Comment on attachment 658358 [details] [diff] [review] Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments, v2 Review of attachment 658358 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, but why not just change IsRootContentDocument()? I don't think there's a good reason to have a function that's wrong in the cross-process case.
For uses like http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDisplayList.cpp#1007, we need a concept of "local root" even if it's not the cross-process root. So we either do this patch or another that creates IsLocalRoot(). I've already written this code, so I guess it's obvious which route I prefer ;).
Comment on attachment 658358 [details] [diff] [review] Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments, v2 Review of attachment 658358 [details] [diff] [review]: ----------------------------------------------------------------- OK.
Attachment #658358 - Flags: review?(roc) → review+
https://hg.mozilla.org/mozilla-central/rev/84fe2b3c5ed9
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: