Get rid of nsIScrollableFrame
Categories
(Core :: Layout: Scrolling and Overflow, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox128 | --- | fixed |
People
(Reporter: TYLin, Assigned: TYLin)
References
(Blocks 1 open bug)
Details
Attachments
(14 files, 1 obsolete file)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
Moved from bug 1824877 comment 1: per https://phabricator.services.mozilla.com/D201893#6924920, we should consider removing nsIScrollableFrame
since ScrollFrame
is the only class implementing nsIScrollableFrame
.
Updated•15 days ago
|
Comment hidden (obsolete) |
Updated•14 days ago
|
Comment hidden (obsolete) |
Assignee | ||
Comment 3•5 days ago
|
||
In theory, changing the return type from nsIFrame*
to ScrollContainerFrame*
exposes ScrollContainerFrame
to the callers who might not needed, but almost
all of the callers in cpp files are already exposed to nsIScrollableFrame
, as
demonstrated in this patch via replacing the #include from
"nsIScrollableFrame.h" to "ScrollContainerFrame.h", so this is OK.
Some callers can be simplified since we no longer need do_QueryFrame
to
nsIScrollableFrame
.
Assignee | ||
Comment 4•5 days ago
|
||
We are going to switch callers from nsIScrollableFrame
to
ScrollContainerFrame
, so we need ScrollContainerFrame
to have the same
interface as nsIScrollableFrame
, including the same default values on params
and the same access privilege of public/protected/private APIs.
This patch is a preparation for later parts so that the code builds
successfully.
Assignee | ||
Comment 5•5 days ago
|
||
Also, simplify some callers of GetScrollTargetFrame()
to drop
nsIScrollableFrame*
and unnecessary do_QueryFrame
. We'll continue removing
more nsIScrollableFrame*
in later parts.
Assignee | ||
Comment 6•5 days ago
|
||
This patch starts from renaming PresShell::GetScrollableFrameToScroll()
to
PresShell::GetScrollContainerFrameToScroll()
, making it return
ScrollContainerFrame*
, and then fixing other helpers until everything
compiles. As before, we can remove some do_QueryFrame
.
Assignee | ||
Comment 7•5 days ago
|
||
This is a preparation to remove #include "nsIScrollableFrame.h"
for
ScrollTimeline.cpp and ViewTimeline.cpp.
Move the implementation of nsListControlFrame::GetMultiple()
to cpp. Keeping
it inline requires adding #include "mozilla/dom/Element.h"
in the header, and
I don't feel the method is hot enough to be inlined.
Assignee | ||
Comment 8•5 days ago
|
||
Convert nsIScrollableFrame
to ScrollContainerFrame
for all the APIs in
nsLayoutUtils, and then adapt other callers until everything compiles.
In nsLayoutUtils::CalculateBasicFrameMetrics()
's documentation,
s/ComputeFrameMetrics/ComputeScrollMetadata/ because the method was renamed in
https://hg.mozilla.org/mozilla-central/rev/cb2023f50288
Assignee | ||
Comment 9•5 days ago
|
||
PresShell::GetRootScrollFrameAsScrollable()
is equivalent to
PresShell::GetRootScrollContainerFrame()
.
In ScrollContainerFrame.h, DecideScrollableLayer()
has two versions, one has
four parameters, and the other has five parameters with the fifth parameter
aDirtyRectHasBeenOverriden
having a default value nullptr
. When we switch
the caller from nsIScrollableFrame
to ScrollContainerFrame
, we need to
remove the default value for the fifth parameter to avoid ambiguity.
Assignee | ||
Comment 10•5 days ago
|
||
Assignee | ||
Comment 11•5 days ago
|
||
Also, in EventStateManager, convert ComputeScrollTarget()
and
ComputeScrollTargetAndMayAdjustWheelEvent()
to return ScrollContainerFrame*
so that we can drop more unnecessarily do_QueryFrame
.
Due to removing nsIScrollableFrame
forward declaration in DOM headers, we have
to add nsIScrollableFrame
forward declaration temporarily in
nsMenuPopupFrame.h to make this patch compile, but we'll remove it once we
remove nsIScrollableFrame
under layout/.
Assignee | ||
Comment 12•5 days ago
|
||
Assignee | ||
Comment 13•5 days ago
|
||
Assignee | ||
Comment 14•5 days ago
|
||
The motivation of this patch is that we've now switched all the callers from
nsIScrollableFrame
to ScrollContainerFrame
, and it is better to audit the
access control of ScrollContainerFrame
's interface, and I notice almost all
the member variables are public!
This patch shuffle the interface so that there are only one public, protected,
and private section, and all the member variables are moved to the private
section.
AutoMinimumScaleSizeChangeDetector
and RemoveDisplayPortCallback
need to
access member variables, so by declaring them under ScrollContainerFrame
, they
have the privilege to do so.
Ideally, the caller only need to access what are already in
nsIScrollableFrame
, so currently we might expose more methods than necessary.
We can audit that in a later part.
Assignee | ||
Comment 15•5 days ago
|
||
This patch moves the documentation from nsIScrollableFrame.h
to
ScrollContainerFrame.h
. We keep methods in the original order as they were in
nsIScrollableFrame
.
Assignee | ||
Comment 16•5 days ago
|
||
Move nsIReflowCallback overridden methods to public section since
ScrollContainerFrame public inherits the interface.
Move reflow related helper methods to protected section.
Comment 17•21 hours ago
|
||
Pushed by aethanyc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/ea9ecb543e66 Part 1 - Rename PresShell::GetRootScrollFrame(), and make it return ScrollContainerFrame. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/a2d37b98273c Part 2 - Sync the interface between nsIScrollableFrame and ScrollContainerFrame. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/b765169a7a8f Part 3 - Change GetScrollTargetFrame() to return ScrollContainerFrame. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/2f0817331dbe Part 4 - Convert some nsIScrollableFrame usage in PresShell, nsLayoutUtils, and related helpers. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/717dac08b607 Part 5 - Fix build errors after including ScrollContainerFrame.h under dom/animation/. r=layout-reviewers,boris https://hg.mozilla.org/integration/autoland/rev/42e1bbe0ecb6 Part 6 - Remove nsIScrollableFrame usages in nsLayoutUtils. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/7b481b747b7a Part 7 - Remove PresShell::GetRootScrollFrameAsScrollable(). r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/c964fccd5180 Part 8 - Remove nsIScrollableFrame usages under accessible/. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/997c9249dbed Part 9 - Remove nsIScrollableFrame usages under dom/. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/6f7ab8adfa6e Part 10 - Remove nsIScrollableFrame usages under gfx/. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/f63b0c4335fe Part 11 - Remove nsIScrollableFrame usages under layout/, widget/, and toolkit/. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/3c06f22da72b Part 12 - Tidy up ScrollContainerFrame's interface. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/c8a6b0e526d6 Part 13 - Remove nsIScrollableFrame completely. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/2977ff81a23e Part 14 - Minor adjustment to ScrollContainerFrame interfaces. r=layout-reviewers,emilio https://hg.mozilla.org/integration/autoland/rev/fd6904338812 apply code formatting via Lando
Comment 18•4 hours ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ea9ecb543e66
https://hg.mozilla.org/mozilla-central/rev/a2d37b98273c
https://hg.mozilla.org/mozilla-central/rev/b765169a7a8f
https://hg.mozilla.org/mozilla-central/rev/2f0817331dbe
https://hg.mozilla.org/mozilla-central/rev/717dac08b607
https://hg.mozilla.org/mozilla-central/rev/42e1bbe0ecb6
https://hg.mozilla.org/mozilla-central/rev/7b481b747b7a
https://hg.mozilla.org/mozilla-central/rev/c964fccd5180
https://hg.mozilla.org/mozilla-central/rev/997c9249dbed
https://hg.mozilla.org/mozilla-central/rev/6f7ab8adfa6e
https://hg.mozilla.org/mozilla-central/rev/f63b0c4335fe
https://hg.mozilla.org/mozilla-central/rev/3c06f22da72b
https://hg.mozilla.org/mozilla-central/rev/c8a6b0e526d6
https://hg.mozilla.org/mozilla-central/rev/2977ff81a23e
https://hg.mozilla.org/mozilla-central/rev/fd6904338812
Description
•