If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

[B2G][Browser][PDF]Graphical fragmentation occurs when viewing a pdf via browser

RESOLVED FIXED in Firefox 28, Firefox OS v1.2

Status

()

Core
Graphics
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: gbennett, Assigned: gw280)

Tracking

({regression})

26 Branch
mozilla28
ARM
Gonk (Firefox OS)
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:koi+, firefox26 wontfix, firefox27 wontfix, firefox28 fixed, b2g-v1.2 fixed)

Details

(Whiteboard: permafail)

Attachments

(3 attachments)

(Reporter)

Description

4 years ago
Description:
After going to http://www.irs.gov/pub/irs-pdf/fw9.pdf directed by test case https://moztrap.mozilla.org/manage/case/1652/, the pdf document is graphically fragmented and sometimes copies the header image "Form W-9 (Rev. August ..." upside down onto the PDF. See http://www.youtube.com/watch?v=hPU5MYStRic

Repro Steps:
1) Updated Buri 1.2 mozRIL to Build ID: 20130912040201
2) Open browser
3) Go to http://www.irs.gov/pub/irs-pdf/fw9.pdf
4) Scroll down and up the PDF document.

Actual:
Document doesn't load properly and breaks visually.

Expected:
PDF loads correctly and can be viewed fully.

Environmental Variables
Device: Buri 1.2 mozRIL
Build ID: 20130912040201
Gecko: http://hg.mozilla.org/mozilla-central/rev/a98569f21abe
Gaia: 9ffd2899eb91388f7fc1ce6f7a895a6f5f922c05
Platform Version: 26.0a1

Notes:
Repro frequency: 100%, 5/5
Test Suite Name: Browser
UCID: browser-111
Link to failed test case: https://moztrap.mozilla.org/manage/case/1652/
See attached: http://www.youtube.com/watch?v=hPU5MYStRic
Can you include a logcat?
blocking-b2g: --- → koi?
Keywords: regression, regressionwindow-wanted

Updated

4 years ago
Component: Gaia::Browser → Gaia::PDF Viewer

Updated

4 years ago
QA Contact: sarsenyev

Comment 2

4 years ago
Regression window!

The bug doesn't reproduce on Buri:
Build ID: 20130906040204
Gecko: http://hg.mozilla.org/mozilla-central/rev/ab5f29823236
Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
Platform Version: 26.0a1

The bug does reproduce on Buri:
Build ID: 20130909114657
Gecko: http://hg.mozilla.org/mozilla-central/rev/218d4334d29e
Gaia: aa4180e9286d385fa6b62d236f30fb24cd8b93e9
Platform Version: 26.0a1
QA Contact: sarsenyev

Updated

4 years ago
Keywords: regressionwindow-wanted
Whiteboard: burirun1 → burirun1, burirun2
We don't do a lot of changes to the PDF Viewer codebase, so I'm thinking this is more likely to be a graphics issue. If I'm wrong, please clarify.
Blocks: 902643
Component: Gaia::PDF Viewer → Graphics
Product: Boot2Gecko → Core
Version: unspecified → 26 Branch
milan,bdahl : Any thoughts on this ? Unclear if its a pdf.js or graphics issue. Would additional logcat from QA help here ?
Flags: needinfo?(milan)
Flags: needinfo?(bdahl)
(In reply to bhavana bajaj [:bajaj] from comment #4)
> milan,bdahl : Any thoughts on this ? Unclear if its a pdf.js or graphics
> issue. Would additional logcat from QA help here ?

FWIW - The video gives a strong indicator this is a graphics issue because scrolling shows the text coming together and splitting apart.
gbennett - can you get a logcat for this bug?
Flags: needinfo?(gbennett)
(Reporter)

Comment 7

4 years ago
Created attachment 815495 [details]
PDFgraphicIssueBrowserLOG.txt

The log starts after pressing enter going to http://www.irs.gov/pub/irs-pdf/fw9.pdf and ends after the page loads and scrolling up/down for a few seconds.

Environment Variables
Device: Buri 1.2 Aurora mozRIL
Gaia: 51f3c79ea93bb91d3b12e50b49d203a049a94a9b
SourceStamp: 3f16dc100b1f
BuildID: 20131010004001
Version: 26.0a2
Flags: needinfo?(gbennett)
It's possible the GL context isn't current, or that we're asking for a very large texture and failing poorly.  The problem could be outside of graphics, but graphics is the best bet for now.  George, let's see if this fails on your device as well.  I see it on Unagi.
Assignee: nobody → gwright
Flags: needinfo?(milan)
This could be SkiaGL running out of cache, though I'm not sure - it is SkiaGL related; if I change the gfx.canvas.azure.accelerated preference to false (in gecko/b2g/app/b2g.js), this works correctly for me.
Flags: needinfo?(bdahl)
Almost sounds like a GLContext issue to me, as SkiaGL should have its own, no? If it does have its own, it really shouldn't be able to grab textures from outside its own context, I think. needinfoing pchang, as I'm not sure how B2G is set up context-wise.
Flags: needinfo?(pchang)
On Unagi, I get "out of pmem" message as we try to load this pdf.  This means that hamachi testing is probably not going to properly show this problem at all until all the "graceful fallback" issues are resolved (bug 905882 et al.)
Created attachment 815559 [details]
What it looks like on unagi when out of pmem is properly handled
(In reply to sarsenyev from comment #2)
> Regression window!
> 
> The bug doesn't reproduce on Buri:
> Build ID: 20130906040204
> Gecko: http://hg.mozilla.org/mozilla-central/rev/ab5f29823236
> Gaia: 94e5f269874b02ac0ea796b64ab995fce9efa4b3
> Platform Version: 26.0a1
> 
> The bug does reproduce on Buri:
> Build ID: 20130909114657
> Gecko: http://hg.mozilla.org/mozilla-central/rev/218d4334d29e
> Gaia: aa4180e9286d385fa6b62d236f30fb24cd8b93e9
> Platform Version: 26.0a1

Right - in this regression range is Bug 905227 pref flip that enabled SkiaGL.
Blocks: 905227
(In reply to Milan Sreckovic [:milan] from comment #11)
> On Unagi, I get "out of pmem" message as we try to load this pdf.  This
> means that hamachi testing is probably not going to properly show this
> problem at all until all the "graceful fallback" issues are resolved (bug
> 905882 et al.)

Perhaps unrelatedly, AsyncChannel reports ChannelError around this time.
blocking-b2g: koi? → koi+
See Also: → bug 915010
(In reply to Milan Sreckovic [:milan] on PTO 17-18 Oct from comment #11)
> On Unagi, I get "out of pmem" message as we try to load this pdf.  This
> means that hamachi testing is probably not going to properly show this
> problem at all until all the "graceful fallback" issues are resolved (bug
> 905882 et al.)

I think these are just noise in the log. I set the Skia texture cache to be 2MB in size and no longer see the pmem errors, but the graphical corruption is still present. Interestingly, we were still running out of pmem with a 4MB cache.

The hunt for the offending code continues... :)
Yeah, so it is a gralloc issue, because disabling gralloc entirely causes this issue to go away
I could reproduce this issue on Nexus 4 which is JB code base.
Flags: needinfo?(pchang)
If we fallback to ashmem, we don't have gralloc buffer for app.
Then app may have problem to use GLScreenBuffer because it uses SharedSurfaceGralloc object on b2g.
does the problem happens when "gfx.gralloc.fence-with-readpixels" is true?

http://mxr.mozilla.org/mozilla-central/source/gfx/gl/SharedSurfaceGralloc.cpp#59
Yes, it still occurs when I set that to true
Created attachment 820320 [details] [diff] [review]
patch

Re-configure the texture parameters could solve this problem in my side. 

George, does it work for you? I guess this app didn't call clearRect and there were some garbage on the screen.
It works for me
Is this a security concern though? If the app doesn't do clearrect, then it can screen scrape the screen and grab sensitive info?
Oh, so it's to do with mipmapping, and a mipmapped texture is 30% bigger than a non-mipmapped, so we're addressing outside the bounds of the gralloc buffer when drawing it, hence why we're seeing part of the system UI drawn inside the pdf.js window.

We should definitely take this patch as everywhere else (SkiaGL, GLContext, etc) sets these parameters to these values and we need it to be that way for consistency. I think this particular instance is occurring because we do the fallback to ashmem but don't preserve the texture params when doing so.
(In reply to George Wright (:gw280) from comment #23)
> Is this a security concern though? If the app doesn't do clearrect, then it
> can screen scrape the screen and grab sensitive info?

This is a rare case where it's not actually a security concern, as the app has no way to do readback from the higher mip levels of the backbuffer.
Attachment #820320 - Flags: review+
To expand on what's happening, many drivers access mip levels by just indexing past the end of the data for mip level 0. If we 'cast' something without mips (gralloc buffers) into something which expects mips (texture with mip filtering), we could easily read out-of-bounds, and random other GPU memory.

The solution is just to never allow mips for gralloc-derived textures.
(In reply to George Wright (:gw280) from comment #24)
> We should definitely take this patch as everywhere else (SkiaGL, GLContext,
> etc) sets these parameters to these values and we need it to be that way for
> consistency. I think this particular instance is occurring because we do the
> fallback to ashmem but don't preserve the texture params when doing so.

In my leo device, I didn't see this app fallback to ashmem. It is still able to request gralloc from pmem.

(In reply to Jeff Gilbert [:jgilbert] from comment #26)
> To expand on what's happening, many drivers access mip levels by just
> indexing past the end of the data for mip level 0. If we 'cast' something
> without mips (gralloc buffers) into something which expects mips (texture
> with mip filtering), we could easily read out-of-bounds, and random other
> GPU memory.
> 
> The solution is just to never allow mips for gralloc-derived textures.

Thanks for comment.

I just found the following files may have the same problem, but not sure which call flow will use EGLImageShare. Does it call by non-oop case on b2g?
http://mxr.mozilla.org/mozilla-central/source/gfx/gl/SharedSurfaceEGL.cpp#258

Updated

4 years ago
Flags: needinfo?(jgilbert)
EGLImage shouldn't have the same problem in this case, as all these textures are sourced from the GL driver. We should probably fix this so it never considers mipmaps, though. Can you file a bug on this, and cc me/assign it to me?
Flags: needinfo?(jgilbert) → needinfo?(pchang)

Updated

4 years ago
Whiteboard: burirun1, burirun2 → burirun1, burirun2,burirun3
(In reply to Jeff Gilbert [:jgilbert] from comment #28)
> EGLImage shouldn't have the same problem in this case, as all these textures
> are sourced from the GL driver. We should probably fix this so it never
> considers mipmaps, though. Can you file a bug on this, and cc me/assign it
> to me?


Create bug 932636 for tracking.
Flags: needinfo?(pchang)

Updated

4 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/b2g-inbound/rev/38f366acb59b
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/38f366acb59b
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/8c4f569c8ac0
status-b2g-v1.2: --- → fixed
status-firefox26: --- → wontfix
status-firefox27: --- → wontfix
status-firefox28: --- → fixed

Updated

4 years ago
Whiteboard: burirun1, burirun2,burirun3 → permafail
You need to log in before you can comment on or make changes to this bug.