Closed Bug 1222783 Opened 6 years ago Closed 6 years ago
crash in ns
HTMLFrameset Frame::Reflow in Firefox 42 with particular addons present
This bug was filed from the Socorro interface and is report bp-52c3a6d7-e247-4457-9a36-d24d92151105. ============================================================= Crashing Thread Frame Module Signature Source 0 xul.dll nsHTMLFramesetFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) layout/generic/nsFrameSetFrame.cpp 1 xul.dll nsBlockReflowContext::ReflowBlock(mozilla::LogicalRect const&, bool, nsCollapsingMargin&, int, bool, nsLineBox*, nsHTMLReflowState&, unsigned int&, nsBlockReflowState&) layout/generic/nsBlockReflowContext.cpp 2 xul.dll nsBlockFrame::ReflowBlockFrame(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp 3 xul.dll nsBlockFrame::ReflowLine(nsBlockReflowState&, nsLineList_iterator, bool*) layout/generic/nsBlockFrame.cpp this nsHTMLFramesetFrame::Reflow crash signature is regressing in firefox 42 and currently on #11 of the top score board. numerous user comments say, this is happening while trying to log into the ing-diba.de (german bank) website, subsequently over 95% of the crashes happen on de locale builds. some other urls are also mentioned looking at correlation data for this signature, it strongly correlates with a particular extension: 68% (69/101) vs. 0% (87/44192) firstname.lastname@example.org https://addons.mozilla.org/firefox/addon/browser-security-1/ and there are also another bunch of obscure security-labelled extensions which seem involved but only exist on a handful of devices overall: email@example.com, Version 4 firstname.lastname@example.org, Version 4 fx@foxysecureKDJJHVLSDUVFU.com, Version 6 email@example.com, Version 6 zz@JDkfjdK, Version 6 firstname.lastname@example.org, Version 4 email@example.com, Version 3 the crash is reproducible: 1. install the addon from https://addons.mozilla.org/firefox/addon/browser-security-1/ 2. visit https://banking.ing-diba.de/app/login, enter some random login data into the form, let the firefox pw manager save it & restart the browser. 3. visit https://banking.ing-diba.de/app/login again. in most cases firefox now crashes on the spot - sometimes it also takes another time. go the homepage https://www.ing-diba.de/ and click on the orange "Log-in Banking" button on the top right for the crash regression testing the issue narrows the window down to the changeset https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=40fb01ba8c9287c5fb982a8cbffdad6862c296fc&tochange=5dcb38c7f1b84d7fc4d0255a2b10165dfaceb941 so i'm also cc'ing dbaron on this bug.
reflow and dbaron's patches are layout, so moving into that component.
Component: General → Layout
Null dereference of mChildBorderColors, presumably because we call nsHTMLFramesetFrame::Reflow twice while NS_FRAME_FIRST_REFLOW is set. I think that's an ok thing to do, and we should just define the firstTime variable in a better way.
I didn't actually test the steps-to-reproduce since they're complicated, and the crash was clear enough from the crash stack. I have a simple testcase (written based on knowing the regressing bug and the stack) that crashes in current builds, and a patch that I believe should fix it that I'll test tomorrow: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/81fdd3806ce9/frameset-set-firsttime-better
Assignee: nobody → dbaron
I confirmed locally that the new crashtest crashes in the harness without the patch, and passes in the harness with the patch.
Attachment #8684912 - Flags: review?(roc)
Attachment #8684912 - Flags: review?(roc) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/76753a9a19fd1043002a4ff8f5babe2682a97eb2 Bug 1222783 - Make nsHTMLFramesetFrame::Reflow set firstTime based on what firstTime means. r=roc
Would you be willing to retest the steps from comment 0 in a nightly from November 12th (which should be out within a few hours) to confirm that this is fixed?
sure, i can confirm that in today's nightly the str from comment #0 don't produce this crash any longer.
Comment on attachment 8684912 [details] [diff] [review] Make nsHTMLFramesetFrame::Reflow set firstTime based on what firstTime means Approval Request Comment [Feature/regressing bug #]: bug 538194 (didn't introduce the bug, but made an existing bug show up more) [User impact if declined]: topcrash #11, involving interaction between extensions and certain pages [Describe test coverage new/current, TreeHerder]: crashtest in tree; manually tested in comment 8 [Risks and why]: pretty low risk; it's just using a more reliable test to make sure we only execute certain code once per object [String/UUID change made/needed]: no
Comment on attachment 8684912 [details] [diff] [review] Make nsHTMLFramesetFrame::Reflow set firstTime based on what firstTime means Given that the fix was verified on Nightly and crash fixes are so important, I feel comfortable taking this in Aurora44.
Attachment #8684912 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 8684912 [details] [diff] [review] Make nsHTMLFramesetFrame::Reflow set firstTime based on what firstTime means Crash fix, recent regression (or worsening of an existing one) Let's take this in beta.
Attachment #8684912 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
this failed to apply to aurora: grafting 314187:76753a9a19fd "Bug 1222783 - Make nsHTMLFramesetFrame::Reflow set firstTime based on what firstTime means. r=roc" merging layout/generic/crashtests/crashtests.list warning: conflicts during merge. merging layout/generic/crashtests/crashtests.list incomplete! (edit conflicts, then use 'hg resolve --mark') merging layout/generic/nsFrameSetFrame.cpp abort: unresolved conflicts, can't continue (use hg resolve and hg graft --continue)
You need to log in before you can comment on or make changes to this bug.