Closed
Bug 1201277
Opened 9 years ago
Closed 9 years ago
Use a lambda instead of a local class as the condition for APZCTreeManager's calls to BreadthFirstSearch()
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
RESOLVED
FIXED
mozilla43
Tracking | Status | |
---|---|---|
firefox43 | --- | fixed |
People
(Reporter: botond, Assigned: michael, Mentored)
Details
(Whiteboard: [lang=c++] [good first bug])
Attachments
(1 file)
2.64 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
APZCTreeManager makes two calls to BreadthFirstSearch(): https://dxr.mozilla.org/mozilla-central/rev/fb720c90eb49590ba55bf52a8a4826ffff9f528b/gfx/layers/apz/src/APZCTreeManager.cpp#1560 https://dxr.mozilla.org/mozilla-central/rev/fb720c90eb49590ba55bf52a8a4826ffff9f528b/gfx/layers/apz/src/APZCTreeManager.cpp#1579 Both calls take a 'condition' argument, which tells the search when to stop (when a node that meets the condition is found). We currently use local classes for both conditions. Using a C++11 lambda would be nicer, but we couldn't do so at the time we wrote this code due to a bug in our static analysis plugin (bug 1170388) that caused a false-positive static analysis error for a use of a lambda there. Now that this static analysis bug is fixed, we should update the calls to use C++11 lambdas. This is a good first bug for someone new to the Mozilla codebase (less so for someone new to C++).
Assignee | ||
Comment 1•9 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d41056e89c89
Assignee | ||
Comment 2•9 years ago
|
||
I've updated APZCTreeManager replacing functors with lambdas where appropriate.
Attachment #8656991 -
Flags: review?(botond)
Reporter | ||
Comment 3•9 years ago
|
||
Comment on attachment 8656991 [details] [diff] [review] Patch Review of attachment 8656991 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, Michael! It looks good. I'm going to do a static analysis build on Try before landing, just in case: https://treeherder.mozilla.org/#/jobs?repo=try&revision=42d1d6cd7083
Attachment #8656991 -
Flags: review?(botond) → review+
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #3) > I'm going to do a static analysis build on Try before landing, just in case: > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=42d1d6cd7083 Looks good, landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/0fe11ce43945
https://hg.mozilla.org/mozilla-central/rev/0fe11ce43945
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → michael
Reporter | ||
Comment 6•9 years ago
|
||
Thanks again, Michael! In case you're interested in working on something a bit more involved in the same part of the codebase, bug 1199798 is concerned with changing some of the other tree traversals in Async Pan/Zoom code to use BreadthFirstSearch() / DepthFirstSearch() instead of a manual loop, and extending these functions to serve the needs of a greater variety of call sites. It's a pretty open-ended bug which doesn't need to be done all at once - you could do however much of it you feel like.
You need to log in
before you can comment on or make changes to this bug.
Description
•