Last Comment Bug 783066 - Gaia homescreen background isn't transparent when it's loaded out of process
: Gaia homescreen background isn't transparent when it's loaded out of process
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Layout (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla18
Assigned To: Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
:
Mentors:
Depends on:
Blocks: b2g-e10s-work 790764
  Show dependency treegraph
 
Reported: 2012-08-15 12:55 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2012-09-12 16:54 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+


Attachments
Hackery (2.55 KB, patch)
2012-09-04 14:28 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
Hackery v2 (881 bytes, patch)
2012-09-04 15:31 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments (4.55 KB, patch)
2012-09-04 17:28 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
no flags Details | Diff | Review
Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments, v2 (7.69 KB, patch)
2012-09-04 21:00 PDT, Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
roc: review+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 12:55:23 PDT
Reproduces on desktop as well.  Might be a platform bug, but let's start off in b2g component.
Comment 1 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 14:16:04 PDT
Vivien fixed or worked around this somehow in gaia, so not a blocker.
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 14:17:13 PDT
False alarm, still a problem.
Comment 3 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 21:16:12 PDT
Vivien did you file a gaia issue for drawing the background in the homescreen itself?
Comment 4 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-15 21:20:36 PDT
Epic bz fail!
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-21 00:38:48 PDT
Vivien, do you have a gaia issue open for loading the background in the homescreen app itself yet?
Comment 6 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-21 03:30:28 PDT
https://github.com/mozilla-b2g/gaia/issues/3639
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-22 04:14:33 PDT
No longer a blocker since we have a better gaia fix.
Comment 8 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-31 20:30:43 PDT
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.
Comment 9 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-08-31 21:04:34 PDT
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?
Comment 10 Timothy Nikkel (:tnikkel) 2012-08-31 23:27:33 PDT
It might be this line that is causing it
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp#4931
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 10:19:31 PDT
Ah!  Thanks for the pointer.
Comment 12 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 13:50:21 PDT
I see us getting to the ColorLayer optimization in FrameLayerBuilder for the nsDisplayCanvasBackground.  Its "mExtraBackgroundColor" is set to rgba(1,1,1,1).
Comment 13 Timothy Nikkel (:tnikkel) 2012-09-04 13:57:52 PDT
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.
Comment 14 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 14:10:20 PDT
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.
Comment 15 Timothy Nikkel (:tnikkel) 2012-09-04 14:19:41 PDT
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.
Comment 16 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 14:28:37 PDT
Created attachment 658233 [details] [diff] [review]
Hackery

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).
Comment 17 Timothy Nikkel (:tnikkel) 2012-09-04 14:35:06 PDT
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.
Comment 18 Timothy Nikkel (:tnikkel) 2012-09-04 14:43:42 PDT
My quick look best guess is it is coming from PresShell::ComputeBackstopColor maybe?
Comment 19 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 15:24:43 PDT
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.
Comment 20 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 15:31:52 PDT
Created attachment 658268 [details] [diff] [review]
Hackery v2

This hackery doo gets rid of the background.
Comment 21 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 15:42:22 PDT
Making PuppetWidget pretend to be transparent doesn't change anything, but I guess that's expected.
Comment 22 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 17:28:35 PDT
Created attachment 658310 [details] [diff] [review]
Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments

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.
Comment 23 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-04 18:02:24 PDT
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?
Comment 24 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 18:08:45 PDT
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 25 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 21:00:20 PDT
Created attachment 658358 [details] [diff] [review]
Paint the backgrounds of remote subdocuments like we paint those of same-process subdocuments, v2
Comment 26 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-04 21:03:40 PDT
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.
Comment 27 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-04 21:08:44 PDT
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 28 Robert O'Callahan (:roc) (Exited; email my personal email if necessary) 2012-09-05 02:50:51 PDT
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.
Comment 29 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2012-09-05 09:24:15 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/84fe2b3c5ed9
Comment 30 Ryan VanderMeulen [:RyanVM] 2012-09-05 19:39:46 PDT
https://hg.mozilla.org/mozilla-central/rev/84fe2b3c5ed9

Note You need to log in before you can comment on or make changes to this bug.