Closed Bug 1161592 Opened 6 years ago Closed 6 years ago

crash in nsLayoutUtils::CalculateAndSetDisplayPortMargins(nsIScrollableFrame*, nsLayoutUtils::RepaintMode)


(Core :: Layout, defect)

Gonk (Firefox OS)
Not set



blocking-b2g 2.5+
Tracking Status
firefox40 --- fixed
b2g-v2.2 --- unaffected
b2g-master --- fixed


(Reporter: onelson, Assigned: kats)



(Keywords: crash, regression, Whiteboard: [3.0-Daily-Testing])

Crash Data


(4 files, 2 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-57160961-b2e7-43d3-aafd-7e07b2150505.

Experienced a crash in the Browser app after searching for 'nuclear throne' and selecting the 3rd link down from Google for the reddit page (''). This was observed only once, and the crash occurred before the new webpage could present itself on screen.

Repro Steps:
1) Update phone to 20150505010204
2) Open Browser
3) Begin search for 'nuclear'
4) Tap 'No Internet Connection' and connect to Wi-Fi
5) Drag down notification tray to kill 'Network Connections' screens
6) Finish search for 'nuclear throne'
7) Select 3rd link down for 'r/NuclearThrone' (reddit page)

Browser closes and feeds user a crash report on screen

Page opens and user may continue browsing
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Whiteboard: [3.0-Daily-Testing]
This was seen once, adding steps-wanted to see if we can find better STR's.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: steps-wanted
So I just reproed this crash after downloading 'Cut the Rope' from marketplace and opening it.


Appears identical, pretty surprising. This app opens in landscape, which kind of plays along with the crash signature, but the original find of this in browser was all done in portrait.
Crashed again opening Settings app after phone display was off for awhile.

Component: Gaia::Browser → Layout
Product: Firefox OS → Core
Adding kats since there is some apz code in the stack.
Flags: needinfo?(bugmail.mozilla)
Does this only show up on 3.0 builds?
blocking-b2g: --- → 3.0+
Keywords: qawanted
I was unable to reproduce this issue on the latest Nightly build.  Leaving steps wanted tag so others can attempt.

Environmental Variables:
Device: Flame 3.0
BuildID: 20150505010204
Gaia: 70077825aab2c7a79611befb40a5fe7e610d5443
Gecko: 102d0e9aa9e1
Gonk: a9f3f8fb8b0844724de32426b7bcc4e6dc4fa2ed
Version: 40.0a1 (3.0) 
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:40.0) Gecko/40.0 Firefox/40.0
Flags: needinfo?(ktucker)
This might be a regression from bug 1156401. Prior to that patch, we would always return early from PrepareForSetTargetAPZCNotification if scrollAncestor was null, but now we can get into the CalculateAndSetDisplayPortMargins call if we don't already have a display port set, and that will crash. We assumed this would never happen (I mention this in bug 1156401 comment 12), but we may have been wrong.
This bug still exist in Message, contacts and Phone... apps in Flame 3.0, can't reproduced in Flame 2.2. After we launch the app, when the page display white-screen, use two fingers to long tap left and right edge at the same time, then crash will pop up.

1.Launch Message.
2.Press Home key to back Home, then launch Contacts.
3.When the page display white-screen, use two fingers to long tap left and right edge at the same time.

Actual result:
3.The Contacts crash will pop up.

Crash reports title:
B2G 40.0a1 Crash Report [@ nsLayoutUtils::CalculateAndSetDisplayPortMargins(nsIScrollableFrame*, nsLayoutUtils::RepaintMode) ]

Crash reports link:

Found time:10:51
See attachment:1051.mp4 and logcat_1051.txt

Device: Flame 2.2 version(Pass):
Build ID               20150505002501
Gaia Revision          772a9491909abd02dc67278dd453746e2dd358a8
Gaia Date              2015-05-05 02:02:24
Gecko Revision
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150505.041743
Firmware Date          Tue May  5 04:18:00 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 version(Fail):
Build ID               20150505010204
Gaia Revision          70077825aab2c7a79611befb40a5fe7e610d5443
Gaia Date              2015-05-04 18:09:33
Gecko Revision
Gecko Version          40.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150505.043622
Firmware Date          Tue May  5 04:36:34 EDT 2015
Bootloader             L1TC000118D0
Attached video 1051.MP4
It's probably what botond said in comment 8. Should be an easy fix, but I want to try to reproduce and confirm.
Assignee: nobody → bugmail.mozilla
Flags: needinfo?(bugmail.mozilla)
I can reproduce this fairly reliably by starting the settings app and then tapping on the screen immediately while it's loading. I use both thumbs to increase the tap rate.
Attached patch Patch (obsolete) — Splinter Review
This is basically the APZCCallbackHelper equivalent of the fix in bug 1161040. We don't want to use the root document element in the child process, only the parent process. This aligns the code with the comment in the function.
Attachment #8602222 - Flags: review?(botond)
See comment 9 or comment 13 for STR. I can also repro using those steps. Also setting 2.2 as unaffected based on comment 9.
QA Whiteboard: [QAnalyst-Triage?]
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Keywords: regression
Comment on attachment 8602222 [details] [diff] [review]

Review of attachment 8602222 [details] [diff] [review]:

::: gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ +508,5 @@
>    // displayport on during initialization.
>    if (nsView* view = nsView::GetViewFor(aWidget)) {
>      if (nsIPresShell* shell = view->GetPresShell()) {
>        MOZ_ASSERT(shell->GetDocument());
> +      if (!shell->GetRootScrollFrame()) {

If there is a root scroll frame, why is scrollAncestor in PrepareForSetTargetAPZCNotification null to begin with?
Hm, that's a good question. It looks like the GetFrameForPoint call is returning a null target, which then gives us a null scrollAncestor. The display list in the GetFrameForPoint call is empty, even though the root frame is non-null. Presumably none of the frames in the document at that point generate display items because they're all transparent or empty or something. I suppose in this case we still want to pick GetRootScrollFrame() as the scrollAncestor and proceed as normal?
Attached patch Alternate patch (obsolete) — Splinter Review
This works too. I guess it boils down to the question "if the user touches into a root content document in the child process with no visible content, should the touch be handled by APZ or not?" and the answer I think is "it doesn't really matter". I think this second patch might be more correct but the first patch makes things more consistent with other parts of the code, so I don't know which is better (or maybe we should do both?)
Attachment #8602904 - Flags: review?(botond)
Comment on attachment 8602904 [details] [diff] [review]
Alternate patch

Review of attachment 8602904 [details] [diff] [review]:

I agree that it doesn't matter, but for consistency with Layout code that special-cases the RCD-RSF, I think it makes sense to do so here too.

::: gfx/layers/apz/util/APZCCallbackHelper.cpp
@@ +475,3 @@
>  {
>    if (!aTarget) {
> +    return aRootFrame->PresContext()->PresShell()->GetRootScrollFrameAsScrollable();

I think this would read better as a check in PrepareForSetTargetAPZCNotification.
Attachment #8602904 - Flags: review?(botond) → review+
Comment on attachment 8602222 [details] [diff] [review]

Review of attachment 8602222 [details] [diff] [review]:

I don't think adding this check makes sense given that the other patch handles the RCD-RSF case specially earlier on in PrepareForSetTargetAPZCNotification.

That said - how confident are we that we'll always have either a scroll frame or a display port on the root element? Would it make sense to make PrepareForSetTargetAPZCNotification more tolerant by returning early if scrollAncestor is null?
Attachment #8602222 - Flags: review?(botond) → review-
This moves the check out into PrepareForSetTargetAPZCNotification and removes the nullcheck from GetScrollableAncestorFrame since that should never happen any more. Carrying r+
Attachment #8602222 - Attachment is obsolete: true
Attachment #8602904 - Attachment is obsolete: true
Attachment #8603397 - Flags: review+
(In reply to Botond Ballo [:botond] [at c++ standards meeting May 4-9] from comment > > That said - how confident are we that we'll always have either a scroll
> frame or a display port on the root element? Would it make sense to make
> PrepareForSetTargetAPZCNotification more tolerant by returning early if
> scrollAncestor is null?

Given that ChromeProcessController always sets a displayport on the root element in the parent process, and child content processes should always have a root scrollframe, I think I'm fairly confident in this. The only bad case would be if we have chrome document at the root of a child process - and if there is an environment where that happens I'd rather we found out about it by crashing rather than letting it sneak by.
See Also: → 1163226
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
Duplicate of this bug: 1163226
[Tracking Requested - why for this release]:
Nightly 41.0a1 crash report

Exit from Nightly(Menu>Exit the program)and go to other window(Explorer or something)>Nightly crash with this crash bug^
You need to log in before you can comment on or make changes to this bug.