Closed
Bug 1161040
Opened 9 years ago
Closed 9 years ago
Possible infinite loop in BuildOverscrollHandoffChain
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: kats, Assigned: kats)
References
Details
Attachments
(2 files, 2 obsolete files)
2.45 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
1.60 KB,
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
Jim reports at https://bugzilla.mozilla.org/show_bug.cgi?id=1157264#c3 that scrolling with the mouse wheel hangs the browser. The crash report there implies an infinite loop in APZCTreeManager::BuildOverscrollHandoffChain, which is something I ran into as well locally. I only saw it once and thought it was just my debugger playing tricks on me, but now I suspect it might be a legitimate regression from bug 1159405. I will investigate.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8600923 -
Flags: review?(botond)
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8600926 -
Flags: review?(tnikkel)
Assignee | ||
Comment 3•9 years ago
|
||
No functional changes intended here but hopefully this makes it easier to follow the different cases.
Attachment #8600927 -
Flags: review?(tnikkel)
Comment 4•9 years ago
|
||
Comment on attachment 8600926 [details] [diff] [review] Part 2 - The actual fix So it seems like the problem is that we will set the same scroll id again when we build display list for the root scroll frame? If that's the case then wouldn't a better fix be to just check if the document has a root scroll frame (that will then set the same scroll id)?
Attachment #8600926 -
Flags: review?(tnikkel)
Updated•9 years ago
|
Attachment #8600927 -
Flags: review?(tnikkel)
Assignee | ||
Comment 5•9 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #4) > So it seems like the problem is that we will set the same scroll id again > when we build display list for the root scroll frame? Yeah. > If that's the case then wouldn't a better fix be to just check if the > document has a root scroll frame (that will then set the same scroll id)? I was considering that but I couldn't convince myself that the logic in nsGfxScrollFrame::BuildDisplayList would always do that because of the conditions at http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsGfxScrollFrame.cpp?rev=ade50933c777#2762. If you think that's a more correct fix I can do that instead.
Comment 6•9 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #5) > (In reply to Timothy Nikkel (:tn) from comment #4) > > If that's the case then wouldn't a better fix be to just check if the > > document has a root scroll frame (that will then set the same scroll id)? > > I was considering that but I couldn't convince myself that the logic in > nsGfxScrollFrame::BuildDisplayList would always do that because of the > conditions at > http://mxr.mozilla.org/mozilla-central/source/layout/generic/ > nsGfxScrollFrame.cpp?rev=ade50933c777#2762. If you think that's a more > correct fix I can do that instead. The code you are changing is in the else branch of an if that checks for ignore viewport scrolling, that should guarantee we don't take that path.
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8600926 -
Attachment is obsolete: true
Attachment #8600927 -
Attachment is obsolete: true
Attachment #8601002 -
Flags: review?(tnikkel)
Updated•9 years ago
|
Attachment #8601002 -
Flags: review?(tnikkel) → review+
Assignee | ||
Comment 8•9 years ago
|
||
I landed part 2 since it should fix the browser hang with APZ. Flagging leave-open for part 1.
Keywords: leave-open
Updated•9 years ago
|
Attachment #8600923 -
Flags: review?(botond) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/376c53019483
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•