Closed Bug 860818 Opened 12 years ago Closed 12 years ago

Not possible to load [e.me] Pinterest

Categories

(Firefox OS Graveyard :: General, defect, P1)

Product:
Firefox OS Graveyard
Component:
General
Platform:
x86_64
Windows 7
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED WORKSFORME

People

(Reporter: dpalomino, Unassigned)

References

Details

(Whiteboard: [MemShrink])

Attachments

(3 files)

adb logcat
25.11 KB, application/vnd.oasis.opendocument.text
Details
memory profile after m.pinterest.com is opened.
1.92 MB, text/plain
Details
loaded image list in m.pinterest.com
13.07 KB, text/plain
Details
See during certification in Spain (adding dep). Buildid "20130321070205", device: ikura gecko commit: b5183c99228bdc5be33340e359efd1b4f0859e92 gaia commit: 577d13088ebdbd353d13910d3317e713a140415b Sometimes, when entering into [e.me] Pinterest, the browser is closed. Nominated tef?
Attached file adb logcat —
blocking-b2g: tef? → tef+
I wonder this is because mozbrowserAPI mistakenly thinks the page is crashed but actually it doesn't.
"http://m.pinterest.com" is causing |AbnormalShutdown| in gecko and then gecko passes fatal mozbrowsererror to gaia. It also crashes the homescreen app.
Change component to General first.
Component: Gaia → General
I tried on my Unagi and it can't be reproduced. CJ, is it something we could help from platform team? Thanks!
Flags: needinfo?(cku)
(In reply to khu from comment #5) > I tried on my Unagi and it can't be reproduced. > CJ, is it something we could help from platform team? Thanks! It "sometimes" occurs. My repro rate is about 85%.
Flags: needinfo?(schien)
Flags: needinfo?(cyu)
Browser app is killed by oom-killer and the memory profile shows that image buffer consume more that 50MB while opening m.pinterest.com. The memory footprint of browser app after successfully opening m.pinterest.com is in below. The peak memory usage during image decoding should be higher than that. The ongoing mem shrink in bug 854783 might help reducing the probability of oom for this scenario. 75.03 MB (100.0%) -- explicit ├──59.69 MB (79.55%) -- images │ ├──59.69 MB (79.55%) -- content │ │ ├──59.69 MB (79.55%) -- used │ │ │ ├──54.45 MB (72.58%) ── uncompressed-heap │ │ │ ├───5.23 MB (06.98%) ── raw │ │ │ └───0.00 MB (00.00%) ── uncompressed-nonheap │ │ └───0.00 MB (00.00%) ++ unused │ └───0.00 MB (00.00%) ++ chrome ├───5.67 MB (07.56%) -- window-objects/top(http://m.pinterest.com/, id=1) │ ├──4.95 MB (06.60%) -- active │ │ ├──4.18 MB (05.58%) -- window(http://m.pinterest.com/) │ │ │ ├──2.42 MB (03.22%) ++ js-compartment(http://m.pinterest.com/) │ │ │ ├──1.00 MB (01.33%) ++ (3 tiny) │ │ │ └──0.77 MB (01.03%) ++ layout
Flags: needinfo?(schien)
Flags: needinfo?(cyu)
Flags: needinfo?(cku)
We have many bugs open about large images crashing Firefox, and the powers that be were made very well aware of these bugs when we decided to use gecko 18 as our base for b2g. We've fixed some of these bugs in recent versions; see bug 689623 and the bugs it blocks. Unfortunately these are large fixes that I don't think we can backport to b2g18. I think we should close this as WONTFIX. If this is actually a release/certification blocker, we have a serious problem on our hands.
Whiteboard: [MemShrink]
tef? see comment 9.
blocking-b2g: tef+ → tef?
Hold up. I don't know if we can WONTFIX this for 1.01. Pinterest is a top site for 1.01 launch (Global = 34, Brazil = 60). I know there was discussion at some point to see if this was going to be a top app, possibly on device with customizations, so I'll redirect to Karen for input here.
Flags: needinfo?(kward)
We'll track bug 862255 instead for now, since: 1) Telefonica only tested this site because it was on e.me 2) This bug represents a high LOE and high risk for v1.0.1 3) Jason is going to see if Pinterest will make a change on their end
blocking-b2g: tef? → -
Depends on: 862255
Flags: needinfo?(kward)
Whiteboard: [MemShrink]
Tom - Can you help with outreach here to talk to Pinterest to fix their site due to this bug?
Flags: needinfo?(telin)
:schien did good investigation here, and we now know that even on a low memory device if you try to open pinterest via browser we believe it would run out of memory and cause some weird behavior. So the app workaround should be providing lower resolution images than just put a big image with only setting the image "width/height" to fit the mobile.
Pinterest is a very popular app. It is not in Marketplace currently but is on the Busness Development Roadmap because it is highly desirable app. It makes sense to ask Tech Evangelism to reach out to Pinterest to ask them to make the lower resolution images available. However, Firefox OS needs to keep the fix on our roadmap so that it resolved.
(In reply to kward@mozilla.com from comment #16) > Pinterest is a very popular app. It is not in Marketplace currently but is > on the Busness Development Roadmap because it is highly desirable app. It > makes sense to ask Tech Evangelism to reach out to Pinterest to ask them to > make the lower resolution images available. However, Firefox OS needs to > keep the fix on our roadmap so that it resolved. Right. Given the high risk of the patch to actually fix the problem though, the best option we've got right now is to outreach. I would greatly appreciate if someone from Business Development can take point to talk to Pinterest about this problem.
I’m speaking with Ron from BD about this now as he’s been chasing Pinterest for a FFOS app. However, Lawrence Mandel (mobile web compat) and his team should be the ones reaching out to them for site corrections. From a BD perspective I think Pinterest will put a low priority on this if it requires any significant amount of engineering work. We should absolutely pursue a platform fix if we can. Does this same crashing issue also happen with higher end (wave 2) launch devices? Those with 512Mb RAM or higher? I’ll post a Pinterest contact to this thread soon.
Flags: needinfo?(telin)
Here's a contact from Ron. Turns out Dave is also an ex-mozillian so he's probably the most sympathetic at person Pinterest. "Dave Dash" <dave@pinterest.com> Again, Lawrence Mandel's web compatibility team is best suited for this task.
Okay, thanks for the contact info. Bouncing over to lmandel for next steps here.
Flags: needinfo?(lmandel)
I use gdb to list all images loaded while opening m.pinterest.com. I log the image url and the corresponding buffer size, i.e. Decoder::mImageDataLength, at Decoder::PostDecodeDone(). The accumulated memory size consumed by decoded image buffer is 40MB before browser app been killed by oom-killer. We should have a memory pool and lazy loading mechanism for image decoding.
> We should have a memory pool and lazy loading mechanism for image decoding. Yes, we should. :) This is bug 689623 and related bugs. We have looked at these bugs and determined that we can't uplift them to b2g18.
(In reply to Justin Lebar [:jlebar] from comment #22) > > We should have a memory pool and lazy loading mechanism for image decoding. > > Yes, we should. :) This is bug 689623 and related bugs. We have looked at > these bugs and determined that we can't uplift them to b2g18. Should we then dupe to bug 689623?
> Should we then dupe to bug 689623? Bug 689623 will not fix this problem on b2g (that was the "and related bugs"). What you want is probably bug 847223, which you'll notice is not even fixed on m-c. If the decision here is "wait for bug 847223 to be fixed", let's not dupe this but instead make this depend on bug 847223. I think that's probably what we should do, but I'm not sure we were totally in consensus on that.
(In reply to Justin Lebar [:jlebar] from comment #24) > > Should we then dupe to bug 689623? > > Bug 689623 will not fix this problem on b2g (that was the "and related > bugs"). What you want is probably bug 847223, which you'll notice is not > even fixed on m-c. > > If the decision here is "wait for bug 847223 to be fixed", let's not dupe > this but instead make this depend on bug 847223. I think that's probably > what we should do, but I'm not sure we were totally in consensus on that. That sounds fine to me.
Depends on: 847223
Also adding memshrink just so this stays on the radar.
Whiteboard: [MemShrink]
@jlebar, thanks a lot for the information you provide. I'll definitely take a look at these related bugs to see if there is any thing I can help.
(In reply to thomas elin from comment #18) > I’m speaking with Ron from BD about this now as he’s been chasing Pinterest > for a FFOS app. However, Lawrence Mandel (mobile web compat) and his team > should be the ones reaching out to them for site corrections. Unless I've missed something, this is not a matter of compatibility. Pinterest isn't doing anything wrong per se. B2G simply can't handle their site. Unless we can show that Pinterest has issues in Firefox for Android and other mobile browers, any change that we request is specifically for B2G. My understanding from the comments is that we want to ask Pinterest to serve smaller images (in terms of file size) to B2G. Has anyone checked to see if smaller images are served to other platforms? Is this to be a request for a Pinterest B2G specific code path? On our end, we need a solution that prevents sites from crashing the home screen (comment 3). However, v1 will not be perfect in terms of content or functionality. Pinterest is clearly popular. I can understand blocking on the home screen crash but I would not block the release on this one site being functional on B2G v1.
Flags: needinfo?(lmandel)
> My understanding from the comments is that we want to ask Pinterest to serve smaller > images (in terms of file size) Close, but not quite. We care about an image's width*height, not its file size. For example, if Pintrest encoded its images with a lower JPEG quality (thus decreasing the file size, but not decreasing the images' resolution), that would not make a dent in this problem. Instead, we want fewer images, or smaller (in terms of width*height) images, or both. > On our end, we need a solution that prevents sites from crashing the home screen > (comment 3). We intentionally kill the homescreen when we're running out of memory, in an attempt to free up memory and prevent the foreground app from dying. Seeing the homescreen die before the browser crashes is expected.
(In reply to Justin Lebar [:jlebar] from comment #29) > > My understanding from the comments is that we want to ask Pinterest to serve smaller > > images (in terms of file size) > > Close, but not quite. We care about an image's width*height, not its file > size. Thank you for the clarification. > > On our end, we need a solution that prevents sites from crashing the home screen > > (comment 3). > > We intentionally kill the homescreen when we're running out of memory, in an > attempt to free up memory and prevent the foreground app from dying. Seeing > the homescreen die before the browser crashes is expected. I read and regurgitated this incorrectly. I had read it as the phone is killed. If the phone doesn't crash, this bug sounds like an extremely nice to have but not a blocker. Jason, comment 13 states that you're going to talk to Pinterest. Have you spoken with them?
Depends on: 862970
Hi, This is a blocker for certification, and I think we just have two options here: a. Fix the issue so pinterest does not make a crash b. remove from e.me listing,as suggested in bug#862255 Renominating to tef? just to be discussed/triaged again. Thanks! David
blocking-b2g: - → tef?
Given that both options (a) and (b) are viable ways to resolve this issue for v1.0.1, we should select option (b) and move on.
(In reply to Lawrence Mandel [:lmandel] from comment #32) > Given that both options (a) and (b) are viable ways to resolve this issue > for v1.0.1, we should select option (b) and move on. Actually, our current plan is (a). See bug 862970.
blocking as bug 862970 is a blocker
blocking-b2g: tef? → tef+
(In reply to Daniel Coloma:dcoloma from comment #34) > blocking as bug 862970 is a blocker I wonder what gaia could do if bug 862970 is fixed. Shouldn't we just dupe this one?
(In reply to Alive Kuo [:alive] from comment #35) > (In reply to Daniel Coloma:dcoloma from comment #34) > > blocking as bug 862970 is a blocker > > I wonder what gaia could do if bug 862970 is fixed. Shouldn't we just dupe > this one? Gaia can't do anything, FWIW. When I asked Justin about this originally, he said he wanted to keep the bug open, retest with bug 862970 if fixed, and close if it is fixed.
QA, could we have your help to verify if this issue is still here with bug 862970? Thanks!
Keywords: qawanted
QA Contact: jsmith
QA Contact: jsmith
Assuming QA contact unassign was due to mid-air.
QA Contact: jsmith
Bumping back to tef? assuming bug 862970 resolves the issue.
blocking-b2g: tef+ → tef?
The fix in bug 862970 did not fix this issue. Pinterest still crashes if scrolled too quickly using the app, as well as in browser. Checked on the master build Unagi Build ID: 20130429031045 Kernel Date: Dec 5 Gecko: http://hg.mozilla.org/mozilla-central/rev/05533d50f2f7 Gaia: 153f6805839ccfcd4b1064568377ef26bacf2960
Keywords: qawanted
> The fix in bug 862970 did not fix this issue. Pinterest still crashes if scrolled too quickly using > the app, as well as in browser. "Crashes if scrolled too quickly" is not the same as "crashes on load", which is what this bug is about. Can you please file a separate bug for "crashes if scrolled too quickly"?
FWIW, on my unagi running a v1.0.1 build from earlier today (gecko fd004d7126, gaia cf2d4136f), I can't reproduce the crash with scrolling in either the browser or e.me pinterest. And I can load m.pinterest.com and the browser (or e.me app) doesn't get killed or crash.
I dug into what happened on comment 42 with a buri device with 1.01 that does not contain the patch along with the comments following yet. Justin is right - this is different bug that's being seen here. What's happening in the fast scrolling case is that we're trying to load many images during scrolling of content that's queued up being loaded by an image decoder. Since Pinterest has lots of images, if you go fast enough, that queue gets large and...we eventually OOM. However, reloading the page leaves the cached images you originally successfully loaded, so you'll be able to get to the Pinterest start page fully loaded as long as you don't do the aggressive scrolling towards images being decoded. I did notice over in https://bugzilla.mozilla.org/show_bug.cgi?id=862255#c12 with b2g18 that on a weak network, scrolling down the page with start queuing up many images to get decoded, so we'll eventually OOM as well in that case. In terms of the definition of "did it load" though, this is wfm. We should continue analyzing other OOM cases here though - the testing on central with the patch reveals we're still not doing so well using Pinterest in certain cases. Based on what I think, the root cause of the OOM is the queue up overflow of trying to decode too many images at one time. Justin - Is what I'm describing above basically bug 847223 or something different?
Status: NEW → RESOLVED
blocking-b2g: tef? → ---
Closed: 12 years ago
Flags: needinfo?(justin.lebar+bug)
Resolution: --- → WORKSFORME
> However, reloading the page leaves the cached images you originally successfully loaded, so you'll > be able to get to the Pinterest start page fully loaded as long as you don't do the aggressive > scrolling towards images being decoded. That may be what it looks like, but that can't be what's actually happening, since when the browser tab crashes, we lose all the decoded images in that tab. > Justin - Is what I'm describing above basically bug 847223 or something different? I'm not sure exactly which patches you do and don't have applied, but in any case bug 847223 is probably not the most relevant bug. In bug 862970 (just landed on b2g18 today), we changed things so that we don't decode all images eagerly when we load a page. So we "fixed" bug 847223. We rely on memory pressure events to throw away decoded images; if those events don't come quickly enough, we can OOM. We can also OOM if we queue up a lot of image decodes and then don't cancel them on memory pressure. That's probably what's happening here. I'm not sure if that answers your question; let me know if it doesn't.
Flags: needinfo?(justin.lebar+bug)
Yup, that addresses my question. Should I file a different bug for the "We can also OOM if we queue up a lot of image decodes and then don't cancel them on memory pressure" issue?
> Should I file a different bug for the "We can also OOM if we queue up a lot of image decodes and > then don't cancel them on memory pressure" issue? Sure. But I think we should keep that separate from the "pintrest crashes when we scroll" bug, because there are other reasons that scrolling pintrest could cause us to crash. For example, it might be that we don't receive memory-pressure notifications quickly enough.
Okay. I filed bug 867264 for the issue.
See Also: → 972130
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: