Closed Bug 975033 Opened 10 years ago Closed 10 years ago

[B2G][Marketplace] Wikipedia displays black screen overlaying content

Categories

(Core :: Panning and Zooming, defect)

28 Branch
ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla30
blocking-b2g 1.3+
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- fixed
b2g-v1.3 --- verified
b2g-v1.3T --- verified
b2g-v1.4 --- verified

People

(Reporter: tnguyen, Assigned: kats)

References

Details

(Keywords: regression, verifyme)

Attachments

(4 files, 3 obsolete files)

Attached image screenshot
Description:
After installing Wikipedia from Marketplace, the app displays correctly. Navigating to "More current events..." and then scrolling down a little bit will result in a black rectangle displayed over the wiki content.

Repro Steps:
1) Updated Buri to BuildID: 20140220040203
2) Navigate to Marketplace and install Wikipedia
3) Launch Wikipedia
4) Scroll down to bottom of page and tap on "More current events..."
5) Once page is completely loaded, scroll down with a few swipes

Actual Result: 
Black overlay is displayed over content of page

Expected Result:
Content of wiki page is displayed properly

Environmental Variables:
Device: Buri Master M-C mozRIL
BuildID: 20140220040203
Gaia: 6e71ab4da1b08586ea0c758edb7aa199ee34cd2f
Gecko: 660b62608951
Version: 30.0a1
v1.2-device.cfg

Attached: screenshot
Does this reproduce on 1.3?
Keywords: qawanted
This issue reproduces on Buri 1.3

Environmental Variables
Device: Buri 1.3 Mozilla
Build ID: 20140218004003
Gecko: https://hg.mozilla.org/releases/mozilla-b2g28_v1_3/rev/b5afe0b10e93
Gaia: df60e9b49207d64da5647ab15760c122adabfba4
Platform Version: 28.0
Firmware Version: v1.2-device.cfg

When the user goes to the Wikipedia app, selects "More Current events...", and scrolls down the page a black overlay is displayed over content on the page.
Keywords: qawanted
QA Contact: jharvey
What about 1.1?
Keywords: qawanted
This issue does not reproduce on 1.2 or 1.1

Environmental Variables:
Device: Buri v1.2 Mozilla
BuildID: 20140220004002
Gaia: 539a25e1887b902b8b25038c547048e691bd97f6
Gecko: 2ea6a65eea23
Version: 26.0
Firmware Version: v1.2-device.cfg

Environmental Variables:
Device: Buri 1.1 Mozilla
BuildID: 20140218041202
Gaia: c5cb252e5d1aa45d14f3a2ea182e73e2271e4496
Gecko: d7f2811943d1
Version: 18.0
Firmware Version: v1.2-device.cfg
Keywords: qawanted
blocking-b2g: --- → 1.3?
Component: General → Graphics: Layers
Product: Firefox OS → Core
Version: unspecified → 28 Branch
QA Contact: jharvey → pfield
This is because the layer size exceeds the max GL texture size. The proper fix is the displayport margins change bug, but I can look into a safer workaround for the shorter term. Also note that bug 965945 will make the wikipedia app crash instead of display the black screen.
Component: Graphics: Layers → Panning and Zooming
Depends on: 957668
Attached patch WIP (based on b2g28) (obsolete) — Splinter Review
Here's a patch that might be good fallback behaviour. Needs more testing and whatnot.
If this goes 1.3- route, please make sure it gets 1.4?/+
NOTES: This issue only occurs with developer option "Enable APZ for all content processes."

This issue started to occur on the Buri 1.3 Build ID: 20131113040200

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20131113040200
Gaia: 7243b75041c93bc7deb240131748d0b164f3f0b0
Gecko: 7b014f0f3b03
Version: 28.0a1
Firmware Version: V1.2-device.cfg


Last working Buri 1.3 Build ID: 20131112040207

Environmental Variables:
Device: Buri 1.3 MOZ
BuildID: 20131112040207
Gaia: a013d01d3a0ddee76f60e61f8306d280c10aafbf
Gecko: 581d180a37f3
Version: 28.0a1
Firmware Version: V1.2-device.cfg
Blocks: gaia-apzc
This patch should also help with bug 961963 (1.4+) so I'll test the patch and see how well it stands up.
Assignee: nobody → bugmail.mozilla
So far it seems good. We'll probably want to drop the repaint interval to 16ms from 40ms to make the fallback path a little nicer, and implementing fling handoff will make it feel a lot better too.
Blocks: 976427
blocking-b2g: 1.3? → 1.4+
This absolutely in no possible way is waiting until 1.4. This is an obvious blocker for a content partner. Back to 1.3?
blocking-b2g: 1.4+ → 1.3?
No longer blocks: 976427
This patch is based on b2g28. Same patch works for master, with getting rid of the nsIPresShell_MOZILLA27 junk. I added a warning but when that gets hit it spews a lot so I'm not sure if we really want it at all. It's just going to flood the output. NS_WARNING only dumps in debug mode so at least this won't affect release builds.
Attachment #8380852 - Attachment is obsolete: true
Attachment #8382251 - Flags: review?(tnikkel)
This improves the behaviour a little bit when we run into this case. We already have the paint throttler so we don't end up with more than one paint inflight, so there should be no real harm to reducing this value a bit more.
Attachment #8382252 - Flags: review?(21)
blocking-b2g: 1.3? → 1.3+
Comment on attachment 8382251 [details] [diff] [review]
Fall back to "apz failed" scrolling when displayport is too big (patch for b2g28)

Review of attachment 8382251 [details] [diff] [review]:
-----------------------------------------------------------------

Actually I just realized we'll probably want wrap this in in an #ifdef B2G block, so it doesn't affect metro. Displayports might legitimately be large there, and I don't think it would run into the gralloc failure because it's not using gralloc.
Attachment #8382251 - Attachment description: Fall back to "apz failed" scrolling when displayport is too big → Fall back to "apz failed" scrolling when displayport is too big (patch for b2g28)
B2G try push with the "master" version of the patch + the repaint interval adjustment is at https://tbpl.mozilla.org/?tree=Try&rev=4ceb814bedda
Comment on attachment 8382251 [details] [diff] [review]
Fall back to "apz failed" scrolling when displayport is too big (patch for b2g28)

Hmm, we make a lot of decisions based on GetDisplayPort. I'm wondering if it might be better to more directly target this at the problem and not create scroll layer items in ScrollFrameHelper::BuildDisplayList if we have a huge displayport. Looking over the uses of GetDisplayPort now to see what makes most sense.
(In reply to Timothy Nikkel (:tn) from comment #17)
> Comment on attachment 8382251 [details] [diff] [review]
> Fall back to "apz failed" scrolling when displayport is too big (patch for
> b2g28)
> 
> Hmm, we make a lot of decisions based on GetDisplayPort. I'm wondering if it
> might be better to more directly target this at the problem and not create
> scroll layer items in ScrollFrameHelper::BuildDisplayList if we have a huge
> displayport. Looking over the uses of GetDisplayPort now to see what makes
> most sense.

Yeah, I think the more targeted change would be better, especially with the understanding that we won't typically hit this with root scroll frames.
Comment on attachment 8382251 [details] [diff] [review]
Fall back to "apz failed" scrolling when displayport is too big (patch for b2g28)

Clearing review until new patch.
Attachment #8382251 - Flags: review?(tnikkel)
New patch for master. I ported it to 1.3 and did some light testing, seems to be ok. Try push for master at https://tbpl.mozilla.org/?tree=Try&rev=6595dadf3df9
Attachment #8382251 - Attachment is obsolete: true
Attachment #8382332 - Attachment is obsolete: true
Attachment #8382648 - Flags: review?(tnikkel)
Blocks: 977423
Comment on attachment 8382648 [details] [diff] [review]
Fall back to "apzc failed" (v2)

Review of attachment 8382648 [details] [diff] [review]:
-----------------------------------------------------------------

::: layout/generic/nsGfxScrollFrame.cpp
@@ +2448,3 @@
>    if (usingDisplayport) {
>      dirtyRect = displayPort;
>    }

If we just use the original ScrollPort size if we exceed the texture limitation, we will lose the apz benefit for high resolution device(ex. nexus 5). We can still enlarge the display port. Just let the size under 4096*4096.
Attachment #8382648 - Flags: review?(tnikkel) → review+
No longer blocks: 977423
I have test the attachment 8382648 [details] [diff] [review] on nexus 5, but the scroll performance is not good in gallery app.
(In reply to Jerry Shih[:jerry] from comment #25)
> I have test the attachment 8382648 [details] [diff] [review] on nexus 5, but
> the scroll performance is not good in gallery app.

Please file a separate bug for this.
No longer blocks: 961963
Comment on attachment 8382648 [details] [diff] [review]
Fall back to "apzc failed" (v2)

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): APZC
User impact if declined: In some cases where scrolling subdocuments are very large, they will cause the app to crash. The crash is intentional, but we can do better. This patch will render the content and it will be usable, although it will scroll more jankily and slowly compared to normal APZ scrolling.
Testing completed: locally
Risk to taking this patch (and alternatives if risky): The patch is pretty targeted so it shouldn't be very high risk. A proper fix for this problem is being worked on in bug 957668, at which point this workaround can be removed.
String or UUID changes made by this patch: none
Attachment #8382648 - Flags: approval-mozilla-b2g28?
Comment on attachment 8382252 [details] [diff] [review]
Reduce paint interval

See above. This patch will help make the scrolling in these fallback scenarios a little bit more smooth by painting more frequently. It has the risk of increasing CPU/power usage in all scrolling scenarios, which may impact how fast other things (e.g. content scripts) are processed.
Attachment #8382252 - Flags: approval-mozilla-b2g28?
Attachment #8382252 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Attachment #8382648 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Keywords: verifyme
https://hg.mozilla.org/mozilla-central/rev/580f0a28f2a6
https://hg.mozilla.org/mozilla-central/rev/0c5652734ad5
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Verified as fixed v1.3. This issue does NOT reproduce on the latest v1.3 Buri build:

3/25 Environmental Variables:
Device: Buri 1.3 MOZ RIL
BuildID: 20140325004002
Gaia: b789780c53adaac199787f51615375f721edfef4
Gecko: 37917eb0dfeb
Version: 28.0
Firmware Version: V1.2-device.cfg

Verified as fixed v1.4. This issue does NOT reproduce on the latest v1.4 Buri build:

3/25 Environmental Variables:
Device: Buri 1.4 MOZ RIL
BuildID: 20140325000201
Gaia: a2a88d0638594a6510f878d2c5e99a6ead7520ad
Gecko: 67bdb575d833
Version: 30.0a2
Firmware Version: V1.2-device.cfg

-

1) I am currently unable to verify this issue on the v1.3T Branch, leaving Verifyme keyword.

2) Changing bug status to verified as this was tested against master v1.4, comment #32, #33
Status: RESOLVED → VERIFIED
Verified on the latest Tarako build

1.3T Environmental Variables:
Device: Tarako 1.3T
BuildID: 20140530014002
Gaia: e68858693b71d917c9c5ee7e215f7ceea04635f7
Gecko: 1945abae19ff
Version: 28.1
Firmware Version: SP6821a-Gonk-4.0-4-29

1.3 tarako: Content of wiki page is displayed properly, no black overlay
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: