Closed
Bug 1450208
Opened 7 years ago
Closed 7 years ago
Crash in static void ExtractRectFromOffset
Categories
(Toolkit :: Find Toolbar, defect)
Tracking
()
VERIFIED
FIXED
mozilla61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox59 | --- | unaffected |
firefox60 | + | verified |
firefox61 | --- | verified |
People
(Reporter: calixte, Assigned: bradwerth)
References
(Blocks 1 open bug)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
59 bytes,
text/x-review-board-request
|
bzbarsky
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
This bug was filed from the Socorro interface and is
report bp-01ddbaf5-8008-491e-8ae5-679800180330.
=============================================================
Top 10 frames of crashing thread:
0 xul.dll static void ExtractRectFromOffset dom/base/nsRange.cpp:2922
1 xul.dll nsRange::CollectClientRectsAndText dom/base/nsRange.cpp:3091
2 xul.dll nsRange::GetClientRects dom/base/nsRange.cpp:3164
3 xul.dll static bool mozilla::dom::RangeBinding::getClientRects dom/bindings/RangeBinding.cpp:1486
4 xul.dll mozilla::dom::GenericBindingMethod dom/bindings/BindingUtils.cpp:3032
5 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:467
6 xul.dll js::ForwardingProxyHandler::call js/src/proxy/Wrapper.cpp:176
7 xul.dll js::InternalCallOrConstruct js/src/vm/Interpreter.cpp:449
8 xul.dll static bool js::jit::DoCallFallback js/src/jit/BaselineIC.cpp:2380
9 @0x16cf60e740e
=============================================================
There are 8 crashes (from 6 installations) in nightly 61 with buildid 20180329222832. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1437509.
[1] https://hg.mozilla.org/mozilla-central/rev?node=0a91126bb0f9
Flags: needinfo?(bwerth)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8963974 -
Flags: review?(bzbarsky)
Reporter | ||
Updated•7 years ago
|
Assignee: nobody → bwerth
Assignee | ||
Comment 3•7 years ago
|
||
Flags: needinfo?(bwerth)
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
![]() |
||
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8963974 [details]
Bug 1450208: Change nsRange::ExtractRectFromOffset to use simpler, hopefully safer logic to determine whether text is vertical.
https://reviewboard.mozilla.org/r/232790/#review238228
r=me though it would be good to have a description in the bug or commit message of what was wrong with the old code...
Attachment #8963974 -
Flags: review?(bzbarsky) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #6)
> Comment on attachment 8963974 [details]
> r=me though it would be good to have a description in the bug or commit
> message of what was wrong with the old code...
I put a note in the commit message, stating that this patch avoids a static_cast that isn't necessary, and could fail under unusual conditions (when isTextFrame() lied).
![]() |
||
Comment 9•7 years ago
|
||
When would IsTextFrame() lie?
Comment hidden (mozreview-request) |
Assignee | ||
Comment 11•7 years ago
|
||
(In reply to Boris Zbarsky [:bz] (no decent commit message means r-) from comment #9)
> When would IsTextFrame() lie?
I don't know if it's even possible, though it's the best explanation I have for the crash signature. The version of the patch I just pushed has MOZ_ASSERT on both pointer parameters, and with the change to eliminate the static_cast, there's no way to do a bad dereference (in debug mode anyway). If nothing else, this will flush out what's causing this crash.
Comment 12•7 years ago
|
||
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/908749c3b858
Change nsRange::ExtractRectFromOffset to use simpler, hopefully safer logic to determine whether text is vertical. r=bz
Comment 13•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
Updated•7 years ago
|
Crash Signature: [@ static void ExtractRectFromOffset] → [@ static void ExtractRectFromOffset]
[@ nsRange::CollectClientRectsAndText]
Reporter | ||
Comment 14•7 years ago
|
||
No more crashes since the patch landed.
Status: RESOLVED → VERIFIED
Comment 16•7 years ago
|
||
bug 1437509 got uplifted to 60.0b14 and this now seems to be a fairly frequent content crash in early stability data. so we probably need to uplift the patch here to beta too.
tracking-firefox60:
--- → ?
Updated•7 years ago
|
Crash Signature: [@ static void ExtractRectFromOffset]
[@ nsRange::CollectClientRectsAndText] → [@ static void ExtractRectFromOffset]
[@ nsRange::CollectClientRectsAndText]
[@ ExtractRectFromOffset ]
Assignee | ||
Comment 17•7 years ago
|
||
Comment on attachment 8963974 [details]
Bug 1450208: Change nsRange::ExtractRectFromOffset to use simpler, hopefully safer logic to determine whether text is vertical.
Approval Request Comment
[Feature/Bug causing the regression]: Bug 1437509
[User impact if declined]: Vertical text may not be findable
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: Has baked in Nightly, avoids a static cast
[String changes made/needed]: none
Attachment #8963974 -
Flags: approval-mozilla-beta?
Comment 18•7 years ago
|
||
Comment on attachment 8963974 [details]
Bug 1450208: Change nsRange::ExtractRectFromOffset to use simpler, hopefully safer logic to determine whether text is vertical.
Regression from beta 14, let's uplift this for beta 15.
Attachment #8963974 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 19•7 years ago
|
||
We should check on this in crash-stats next week after beta 15 is out, to verify.
Flags: qe-verify+
![]() |
||
Comment 20•7 years ago
|
||
bugherder uplift |
Comment 21•7 years ago
|
||
I verified the crash stats after Firefox 60.0b15 was out, there are no crashes from 7 days ago to now, here is the link: https://crash-stats.mozilla.com/signature/?product=Firefox&signature=static%20void%20ExtractRectFromOffset&date=%3E%3D2018-04-19T13%3A05%3A57.000Z&date=%3C2018-04-26T13%3A05%3A57.000Z
Updated•7 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•