Remove duplicate CreateSimpleScrollingLayer() in gfx/layers/apz/test/gtest/TestTreeManager.cpp
Categories
(Core :: Panning and Zooming, task, P3)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox157 | --- | fixed |
People
(Reporter: botond, Assigned: s.giddeon.card, Mentored)
Details
(Keywords: good-first-bug, Whiteboard: [lang=c++])
Attachments
(1 file)
Filing as a good first bug to learn workflows.
The APZ gtests define a helper CreateSimpleScrollingLayer() twice, with byte-identical bodies. The copy in APZCTreeManagerGenericTester (TestTreeManager.cpp) merely shadows the one it inherits from APZCTreeManagerTester (APZCTreeManagerTester.h), so it should be deleted.
Both copies read:
void CreateSimpleScrollingLayer() {
const char* treeShape = "x";
LayerIntRect layerVisibleRect[] = {
LayerIntRect(0, 0, 200, 200),
};
CreateScrollData(treeShape, layerVisibleRect);
SetScrollableFrameMetrics(layers[0], START_SCROLL_ID,
CSSRect(0, 0, 500, 500));
}
The fix is to delete the whole method from APZCTreeManagerGenericTester in TestTreeManager.cpp, keeping the one in APZCTreeManagerTester.h. Since the bodies are identical, every call site keeps behaving exactly as before; they just resolve to the inherited version instead of the shadowing one.
The copy to delete (TestTreeManager.cpp, line 14):
https://searchfox.org/firefox-main/source/gfx/layers/apz/test/gtest/TestTreeManager.cpp#14
The copy to keep (APZCTreeManagerTester.h, line 214):
https://searchfox.org/firefox-main/source/gfx/layers/apz/test/gtest/APZCTreeManagerTester.h#214
To verify the fix:
./mach gtest 'APZ*'
All the tests should still pass. There are nine call sites of CreateSimpleScrollingLayer() across TestTreeManager.cpp, TestPanning.cpp and TestHitTesting.cpp, and the ones in fixtures that don't derive from APZCTreeManagerGenericTester already use the inherited version today, so the build also confirms the signature is reachable from everywhere it needs to be.
Why this is desired: the duplicate is easy to miss, and if someone edits one copy to adjust the layer geometry, tests using the other copy silently keep the old geometry. Removing the shadowing copy makes there be exactly one definition to maintain.
Tutorial to contribute:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
https://firefox-source-docs.mozilla.org/contributing/stack_quickref.html
Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.
| Assignee | ||
Comment 1•1 month ago
|
||
Updated•1 month ago
|
Comment 3•1 month ago
|
||
| bugherder | ||
Updated•17 days ago
|
Description
•