The failure mode in the attached crashtest is an inconsistency in the flattened
tree. Specifically, we null out mVideoControls in an nsVideoFrame, but defer
the UnbindFromTree call on that NAC element, which measn that its mParent still
points to the nsVideoFrame's mContent. Because all this stuff runs off of script
runners, and the anonymous content destroyer is not guaranteed to run before
other potential script runners, we end up running arbitrary script while the
tree mismatch exists. This script calls back into ProcessPendingRestyles, which
causes trouble.
We could build a separate deferral mechanism, but it's not clear that we actually
need to defer the unbind anymore. The deferred unbind was added in bug 489008,
which predated a lot of simplifications in layout/dom interaction.
MozReview-Commit-ID: 1JYAhiXKVJC
This ensures that if the scroll event triggers style changes, they are
reflected on the same paint.
This is accomplished by having the refresh driver fire scroll events as
an explicit step after FlushType::Style observers and rAF callbacks, and
before the actual style flush.
MozReview-Commit-ID: 4kgauD5SgVo
--HG--
extra : rebase_source : 5f2c869c0749c1e1473797f2e202c075907a45fd
The pending transforms must have been computed using the older scroll offset
values, which means that updating the scroll offsets without recomputing the
transforms will make them wrong. If we do an empty transaction for the scroll
offset updates, the transforms will not get computed. This patch catches this
scenario and schedules a full paint instead of the empty transaction instead.
The case where the scroll offset is modified *before* the transform is already
handled by code in nsIFrame::TryUpdateTransformOnly.
MozReview-Commit-ID: I5s5J7BS1ru
--HG--
extra : rebase_source : 63fec656440c8bee322f069a4466a311ebcd0f7d
This fixes an incremental layout bug, where the number of times we
reflow the frame affects its layout. This is because we make the
decisions about the vertical scrollbar before the horizontal scrollbar
(and, when making the decision, adjust mHelper.mScrollPort for the size
of the scrollbar). Thus, in order to avoid a situation where reflowing
the scrollframe once leads us to have no vertical scrollbar, but
reflowing it a second time leads us to add that scrollbar, we need to
retest for the need for a vertical scrollbar after we add the horizontal
one.
(It's possible there are some other missing cases here, but this is the
one that (a) already existed in the code and (b) is needed to fix the
reftest failure on Windows that I got on bug 1308876, in
layout/reftests/text-overflow/xulscroll.html .
The reftest here shows the bug even without bug 1308876 (though I
confirmed that only by loading the test and reference in a nightly
build, not in the reftest harness). I did test that, in combination
with bug 1308876, the test fails without the patch and passes with the
patch.
MozReview-Commit-ID: LhMi7LbmB6J
ScrollFrameHelper::DecideScrollableLayer doesn't get the actual value of usingDisplayPort/wasUsingDisplayPort if we are not painting to the window. It then sets mWillBuildScrollableLayer to usingDisplayPort. mWillBuildScrollableLayer is the value that determines if we are active and hence an ASR, we don't want to change that between paint display lists for an event handling display list.
We then want to condition uses of mWillBuildScrollableLayer and the return value of DecideScrollableLayer on if we want painting to the window so we get the same behaviour as before when not painting to the window.
This patch does the following renamings, which increase consistency.
- GeckoProfilerInitRAII -> AutoProfilerInit
- GeckoProfilerThread{Sleep,Wake}RAII -> AutoProfilerThread{Sleep,Wake}
- GeckoProfilerTracingRAII -> AutoProfilerTracing
- AutoProfilerRegister -> AutoProfilerRegisterThread
- ProfilerStackFrameRAII -> AutoProfilerLabel
- nsJSUtils::mProfilerRAII -> nsJSUtils::mAutoProfilerLabel
Plus a few other minor ones (e.g. local variables).
The patch also add MOZ_GUARD_OBJECT macros to all the profiler RAII classes
that lack them, and does some minor whitespace reformatting.
--HG--
extra : rebase_source : 47e298fdd6f6b4af70e3357ec0b7b0580c0d0f50
nsIFrame::mClass is of type enum class nsQueryFrame::ClassID which is
a strict subset of the nsQueryFrame::FrameIID values. For a concrete
frame class, its FrameIID is the same numeric value as its ClassID.
MozReview-Commit-ID: 1N0AkCGo1ol
In bug 1359868 we started to do this, but we bounded the pre-render region for
the entire scrollbar by the widget bounds, which is not helpful for tall
scrollframes with short thumbs.
This time, we are bounding the pre-render region of the thumb only, so a small
thumb will always be completely painted.
MozReview-Commit-ID: 5LuP5Lfahdm
--HG--
extra : rebase_source : 3ab45f979160d7991aec71020cf57c9a1e57d1ce
This avoids conflicts with mozilla::dom::FrameType.
MozReview-Commit-ID: 7aEMbHRaTFk
--HG--
extra : rebase_source : 2d01321f5ce0ec8c0e3f70984674f82678034b3c
ScrollFrameActivityTracker::NotifyExpired() will be invoked by
nsExpirationTracker::TimerCallback() from an unlabeled runnable.
We provide a SystemGroup EventTarget for the invocation of this
callback since there's nothing within a page that would rely on
the timer firing at a particular time (i.e., it doesn't matter
when this timer's callback is scheduled, relative to other
runnables dispatched for the page).
MozReview-Commit-ID: 9QEjxCtFhve
--HG--
extra : rebase_source : 06b979835363b9c4288dd218d2a4ca2dc111169b
NotifyApproximateFrameVisibilityUpdate gets the displayport so we want the base rect set before calling it.
We also don't want to record the displayport if we ignored it in the actual visibility pass.
If we disable APZ on an individual scrollable element by setting the "disable APZ"
flag on the ScrollMetadata, we should also disable paint-skipping for that element.
If we don't do this, we end up in a situation where the APZ code is not applying
the async transform but is sending repaint requests expecting the main thread to
do repaints. Meanwhile the main-thread thinks that it can send empty transactions
and have APZ update the async transform (a.k.a. paint-skipping). So visually
neither APZ nor main-thread have an effect and the element doesn't appear to
scroll except with the tile-aligned displayport shifts. Disabling paint-skipping
on the element fixes the issue.
MozReview-Commit-ID: H4wpVLw8r8X
The scroll frame is almost always the content's primary frame and if so
it already has the correct style values and the nsFrame ctor has set
mWritingMode correctly based on those. For the edge cases where it's
not the primary frame, e.g. <fieldset style=overflow:scroll>, the UA
sheet specifies 'inherit' for the relevant properties so it has
the correct style values in this case too.
MozReview-Commit-ID: 1FMFNfF0IqU
This adds markers for FireScrollEvent, DispatchSynthMouseMove, ForwardTransaction, and NotifyDidPaint.
MozReview-Commit-ID: 9XahFGzIOls
--HG--
extra : rebase_source : 1985b2a11777a17dac621a35632f138a76224b0f
This patch is written by the following script with some manual adjustment to
the comment in nsRubyTextContainerFrame.cpp and nsRubyFrame.cpp, and
nsColumnSetFrame's constructor.
function rename() {
find layout\
-type f\
\( -name "*.cpp" -or\
-name "*.h" \)\
-exec sed -i -r "s/$1/$2/g" "{}" \;
}
rename "nsReflowStatus *([a-zA-Z0-9]*) = NS_FRAME_COMPLETE" "nsReflowStatus \1"
rename "([a-zA-Z0-9.*]*) *= NS_FRAME_COMPLETE;" "\1.Reset();"
rename "([a-zA-Z0-9.*]*) == NS_FRAME_COMPLETE" "\1.IsEmpty()"
MozReview-Commit-ID: 9tqQAHvdQex
--HG--
extra : rebase_source : 3119776946dc2c8350098b7bf9f3ceff29bdffb5
Root frame containers really throw a wrench into the whole system.
MozReview-Commit-ID: 9066vWMYxxr
--HG--
extra : rebase_source : 7e79c1ffa1a327e0ee9f780885e2fae02b8846ed
This is the bulk of the changes.
- DisplayItemScrollClip is removed. Instead, we will have 1) ActiveScrolledRoot
and 2) DisplayItemClipChain.
- ActiveScrolledRoot points to a scroll frame and allows traversing up the
scroll frame chain.
- DisplayItemClipChain is a linked list of clips, each clip being associated
with the ActiveScrolledRoot that moves this clip.
- Each display item has an ActiveScrolledRoot and a clip chain.
- nsDisplayItem::GetClip returns the item of the clip chain that scrolls with
the item's ASR. The separation between "regular clip" and "scroll clips"
mostly goes away.
- Tracking clips in the display list builder's clip state happens very
similarly to how regular clips used to be tracked - there's a clip chain for
content descendants and a clip chain for containing block descendants. These
clip chains are intersected to create the combined clip chain.
- There are strict rules for the ASR of a container item: A container item's
ASR should be the innermost ASR which the item has finite clipped bounds with
respect to.
- At some point in the future, ASRs and AGRs should be reunified, but I haven't
done that yet, because I needed to limit the scope of the change.
MozReview-Commit-ID: KYEpWY7qgf2
--HG--
extra : rebase_source : c727f6300a35463750639e165bfa37374c06b851
If, within a single refresh driver tick, the scroll position is updated by JS
explicitly, and then subsequently also updated by a frame reconstruction, the
scroll origin from the former (nsGkAtoms::other) can get clobbered by the latter
(to nsGkAtoms::restore). The restore scroll origin is "weaker" in that it can
be ignored by the APZ code in some circumstances. This is undesirable because
it means the JS scroll update also gets ignored. This patch ensures that when
setting the scroll origin we don't do this clobbering of stronger origins with
weaker origins.
MozReview-Commit-ID: DA4EHp1Debu
--HG--
extra : rebase_source : 99fd1f91698a605792b2a622450f1ff31bc89101
Previously we weren't sending scroll position updates with origin nsGkAtoms::restore
over to the APZ at all, on the assumption that they should never clobber an APZ
scroll offset. However, there are scenarios where that is not true.
In particular, during a frame reconstruction, a layers update may be sent to the
compositor between the time a scrollframe has RestoreState() called on it, and
the time the scrollframe has ScrollToRestoredPosition() called on it. The layers
update that happens during this interval (correctly) sends a scroll position of
(0,0), and forces the APZ to scroll to that position. This is necessary to
prevent APZ from remaining at an invalid scroll offset while the frame is still
being rebuilt.
However, once ScrollToRestoredPosition() is called and the old scroll offset is
restored, that restored scroll position needs to get sent to the APZ in order to
have it properly restore to the original scroll position. In order to do this,
the main thread must flag the metrics with a scroll offset update. Since the user
may have scrolled concurrently in the compositor from the (0,0) position, we also
need to check for that case in the APZ code and avoid restoring the scroll
position. This is equivalent to the corresponding main-thread code in
ScrollToRestoredPosition().
MozReview-Commit-ID: LxRapVSrsJ3
The patch is generated from following command:
rgrep -l unused.h|xargs sed -i -e s,mozilla/unused.h,mozilla/Unused.h,
MozReview-Commit-ID: AtLcWApZfES
--HG--
rename : mfbt/unused.h => mfbt/Unused.h
This misspelling was introduced in bug 1125767, changeset b9951cca6d1f.
MozReview-Commit-ID: KQNlLelY2Kn
--HG--
extra : rebase_source : 7b2b8379da23b06737b462dd4c316b5758d807a9
We want the maximum scroll position to be aligned with layer pixels. That way
we don't have to re-rasterize the scrolled contents once scrolling hits the
edge of the scrollable area.
Here's how we determine the maximum scroll position: We get the scroll port
rect, snapped to layer pixels. Then we get the scrolled rect and also snap
that to layer pixels. The maximum scroll position is set to the difference
between right/bottom edges of these rectangles.
Now the scrollable area is computed by adding this maximum scroll position
to the unsnapped scroll port size.
The underlying idea here is: Pretend we have overflow:visible so that the
scrolled contents start at (0, 0) relative to the scroll port and spill over
the scroll port edges. When these contents are rendered, their rendering is
snapped to layer pixels. We want those exact pixels to be accessible by
scrolling.
This way of computing the snapped scrollable area ensures that, if you scroll
to the maximum scroll position, the right/bottom edges of the rendered
scrolled contents line up exactly with the right/bottom edges of the scroll
port. The scrolled contents are neither cut off nor are they moved too far.
(This is something that no other browser engine gets completely right, see the
testcase in bug 1012752.)
There are also a few disadvantages to this solution. We snap to layer pixels,
and the size of a layer pixel can depend on the zoom level, the document
resolution, the current screen's scale factor, and CSS transforms. The snap
origin is the position of the reference frame. So a change to any of these
things can influence the scrollable area and the maximum scroll position.
This patch does not make us adjust the current scroll position in the event
that the maximum scroll position changes such that the current scroll position
would be out of range, unless there's a reflow of the scrolled contents. This
means that we can sometimes render a slightly inconsistent state where the
current scroll position exceeds the maximum scroll position. We can fix this
once it turns out to be a problem; I doubt that it will be a problem because
none of the other browsers seems to prevent this problem either.
The size of the scrollable area is exposed through the DOM properties
scrollWidth and scrollHeight. At the moment, these are integer properties, so
their value is rounded to the nearest CSS pixel. Before this patch, the
returned value would always be within 0.5 CSS pixels of the value that layout
computed for the content's scrollable overflow based on the CSS styles of the
contents.
Now that scrollWidth and scrollHeight also depend on pixel snapping, their
values can deviate by up to one layer pixel from what the page might expect
based on the styles of the contents. This change requires a few changes to
existing tests.
The fact that scrollWidth and scrollHeight can change based on the position of
the scrollable element and the zoom level / resolution may surprise some web
pages. However, this also seems to happen in Edge. Edge seems to always round
scrollWidth and scrollHeight upwards, possibly to their equivalent of layout
device pixels.
MozReview-Commit-ID: 3LFV7Lio4tG
--HG--
extra : rebase_source : 3e4e0b60493397e61283aa1d7fd93d7c197dec29
extra : source : d43c2d5e87f31ff47d7f3ada66c3f5f27cef84a9
We want the maximum scroll position to be aligned with layer pixels. That way
we don't have to re-rasterize the scrolled contents once scrolling hits the
edge of the scrollable area.
Here's how we determine the maximum scroll position: We get the scroll port
rect, snapped to layer pixels. Then we get the scrolled rect and also snap
that to layer pixels. The maximum scroll position is set to the difference
between right/bottom edges of these rectangles.
Now the scrollable area is computed by adding this maximum scroll position
to the unsnapped scroll port size.
The underlying idea here is: Pretend we have overflow:visible so that the
scrolled contents start at (0, 0) relative to the scroll port and spill over
the scroll port edges. When these contents are rendered, their rendering is
snapped to layer pixels. We want those exact pixels to be accessible by
scrolling.
This way of computing the snapped scrollable area ensures that, if you scroll
to the maximum scroll position, the right/bottom edges of the rendered
scrolled contents line up exactly with the right/bottom edges of the scroll
port. The scrolled contents are neither cut off nor are they moved too far.
(This is something that no other browser engine gets completely right, see the
testcase in bug 1012752.)
There are also a few disadvantages to this solution. We snap to layer pixels,
and the size of a layer pixel can depend on the zoom level, the document
resolution, the current screen's scale factor, and CSS transforms. The snap
origin is the position of the reference frame. So a change to any of these
things can influence the scrollable area and the maximum scroll position.
This patch does not make us adjust the current scroll position in the event
that the maximum scroll position changes such that the current scroll position
would be out of range, unless there's a reflow of the scrolled contents. This
means that we can sometimes render a slightly inconsistent state where the
current scroll position exceeds the maximum scroll position. We can fix this
once it turns out to be a problem; I doubt that it will be a problem because
none of the other browsers seems to prevent this problem either.
The size of the scrollable area is exposed through the DOM properties
scrollWidth and scrollHeight. At the moment, these are integer properties, so
their value is rounded to the nearest CSS pixel. Before this patch, the
returned value would always be within 0.5 CSS pixels of the value that layout
computed for the content's scrollable overflow based on the CSS styles of the
contents.
Now that scrollWidth and scrollHeight also depend on pixel snapping, their
values can deviate by up to one layer pixel from what the page might expect
based on the styles of the contents. This change requires a few changes to
existing tests.
The fact that scrollWidth and scrollHeight can change based on the position of
the scrollable element and the zoom level / resolution may surprise some web
pages. However, this also seems to happen in Edge. Edge seems to always round
scrollWidth and scrollHeight upwards, possibly to their equivalent of layout
device pixels.
MozReview-Commit-ID: 3LFV7Lio4tG
--HG--
extra : histedit_source : 5390eeebfe9a2791d9ac8e91ec1dfec4ec7b4118
It may be that when the frame is reconstructed after load, the frame gets shorter,
and the old scroll position cannot be restored, because it is out of bounds. In
such a case, we don't want to keep mRestorePos tracking the old scroll position,
because it can get incorrectly applied on a future frame reconstruction. Instead,
for scroll position restorations during frame reconstructions, we just try the
restore once and then clear mRestorePos.
MozReview-Commit-ID: BHoJHz0mGmf
Written purely with sed, over .h and .cpp files in layout/.
(While this wasn't explicitly reviewed, I'm considering it as r=dholbert
based on the request in comment 47 in the bug.)
MozReview-Commit-ID: 6Q0F4ViOyjJ
This is a manual subset of changes written with sed, over .h and .cpp
files in layout/.
This also renames a static method on nsSprocketLayout.
Note that nsFlexContainerFrame and nsRangeFrame also have IsHorizontal
methods that are not renamed here, but this can be found to be
relatively safe because none of the IsHorizontal methods are virtual.
MozReview-Commit-ID: Jsdy7I4Q7mX
This is a manual subset of changes written with sed, over .h and .cpp
files in layout/. It's a subset because there is also a Selection
method called IsCollapsed, which is not changed here.
MozReview-Commit-ID: 9JgnPv0Hkff
nsLayoutUtils::CalculateCompositionSizeForFrame() is not affected by the
document resolution when called for the root content document's root
scroll frame. When determining the root composition bounds in order to
calculate a displayport base, if the frame used is the RCD-RSF we must
be sure to scale the bounds ourselves by the document resolution.
MozReview-Commit-ID: ATctmEYvWIJ
--HG--
extra : rebase_source : ba96e7ecc2cefdbeacb08fbd3f5817819dd933c8
Bug 1241917 made it so that a subframe's displayport base is restricted
to the root composition bounds (in addition to its previous
restrictions). This involved an expensive coordinate transformation
causing a scrolling performance regression.
This avoids restricting the displayport base to the root composition
bounds unless the frame has a display port, avoiding the expensive
computation unless necessary.
MozReview-Commit-ID: FVacUscAfu2
--HG--
extra : transplant_source : %F9%E9%19%06/%9C%EA%8C%D1%D5%BD%ED%26C%97y%15%92%7E%CB
It's not obvious that it does this (unless you read the comment or the code), and we don't gain much by doing it.
Also we need to split it up for the next patch in this bug.
Previously displayport bases were computed as the intersection of the
scrollport with the dirtyrect. However the dirtyrect covers what is
rendered, and with displayports what we render can be much larger than
what is visible. With displayport bases intended to represent what was
visible, this was a problem. By restricting them to the root composition
size this makes them more closely match what is visible. To do this more
properly we'd want to intersect the dirtyrect with the scroll clip of
every ancestor scroll frame, not just the root composition bounds.
Note that nsMathMLContainerFrame and its subclasses are unchanged since
they are not target of fullscreen (and thus no backdrop frame), and they
have an assertion to ensure we really don't pass any unexpected list in.
--HG--
extra : source : a1f7ff18a69cc116120de33f14ae62f576a4b55a
Displayport margins change by small amounts on almost every single scroll. We do not want to update image visibility nearly that often.
As the comment, and the original bug (bug 1169881) suggest this is only meant to catch rather large changes in display ports as we already have means to trigger an image visibility update via a scroll position change and via any style or layout flush.
This is a regression from bug 1002992 where we switch from the display list builder to the frame tree walker and didn't update mLastUpdateImagesPos in the frame walker.
Instead do it when we first encounter the root scroll frame.
Doing this goes back to bug 635053, where we did it because fixed position items weren't getting included. However in bug 974643 we learned that this was wrong. Displayports aren't relevant to fixed pos content, displayports are only relevant to scrolled content. And we set the dirty rects of fixed pos content specially. The only other thing that should be affected is scrollbars, and we already carefully set their dirty rects too.
Temporarily rename GetDisplayPort to GetDisplayPortRelativeToScrollPort for the duration of this patchset.
This means that every caller of GetDisplayPort is guaranteed to be touched by this patchset (assuming it compiles), and thus each call site can be checked in review to make sure it is relative to the correct coordinate system.
This also has the side effect of making the computation that takes place in GetDisplayPortFromMarginsData happen relative to the scrollport. As opposed to some things relative to the scroll port, and some things relative to the scroll frame.
This makes the one caller that needs the displayport rect have to ask for it seperately.
The reason for this is later in the patch series we need to add "RelativeToScrollFrame/Port" to all displayport getters, but there is no semantically good way to do that to the name GetOrMaybeCreateDisplayPort.
This makes it clearer whether we're just checking if a displayport exists,
or we're actually consuming its value.
--HG--
extra : source : 70bb222e860669a6cf6e803dbd57f088ca4fbc04
With APZ we want to be firing scroll events to content more consistently, so
we tie them to the refresh driver tick rather than firing them on paint or
haphazardly on the next spin of the event loop.
Patch by Markus Stange, test fixes by Kartikaya Gupta
--HG--
extra : commitid : 7nnkRC8afAJ
The only reason we had this in the scrollframe at all was so that it could be
saved/restored as part of the frame state when leaving a page and then going
back to it. However we can accomplish this by just reading/writing the resolution
from/to the presshell instead, so there's no need to keep a second copy of it.
--HG--
extra : commitid : J4QBfG2GGjn
