Bug 1551659 - Remove MVMContext::ResizeEventFlag and related code. r=botond,hiro

D46944 / bug 1583534 is what fixes the root cause of bug 1528052 by not
having the first call to ResizeReflow have a wrong old size of 0x0.

This removes the code that bug introduces to suppress resize events, which
fixes this bug. I think our behavior now is pretty sane.

In particular, consider the test-case:

<!doctype html>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<a href="" target="_blank">Open me in a separate tab</a>
<pre id="log"></pre>
<script>
  // This shouldn't be needed, but otherwise Fenix doesn't show the tooltip on
  // longpress...
  document.querySelector("a").href = location.href;

  function logSize() {
    log.innerText += window.innerWidth + "x" + window.innerHeight + "\n";
  }
  logSize();
  onresize = logSize;
</script>

(Hosted at https://crisal.io/tmp/gecko-mobile-resize.html for convenience)

Right now on trunk, when you click the link from GVE or Fenix, we're only
getting an initial size of 0x0 (which is not great, btw), and only after first
paint we get the real device size, but content doesn't get a resize event.

This is obviously wrong, every time the layout viewport changes we should fire
resize events.

Pages that get opened in new tabs and get refreshed when resized may get an
extra reload with this approach, but this seems not avoidable unless widget sets
the viewport size right in advance (which from discussion with snorp and agi
doesn't seem possible in the general case).

What used to happen is that we were triggering a redundant resize reflow from
the initial paint which didn't update the layout viewport (because the content
viewer and co had the right viewport from the previous navigation).

Now that we optimize those away, I think our behavior should be correct.

Differential Revision: https://phabricator.services.mozilla.com/D46956

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2019-09-25 19:35:29 +00:00
parent d8374938eb
commit 161cb16ca8
7 changed files with 11 additions and 30 deletions

View File

@ -82,7 +82,7 @@ class MockMVMContext : public MVMContext {
mResolution = aResolution;
mMVM->ResolutionUpdated(aOrigin);
}
void Reflow(const CSSSize& aNewSize, ResizeEventFlag) {
void Reflow(const CSSSize& aNewSize) {
mICBSize = aNewSize;
mContentSize = mLayoutFunction(mICBSize);
}

View File

@ -176,20 +176,14 @@ void GeckoMVMContext::UpdateDisplayPortMargins() {
}
}
void GeckoMVMContext::Reflow(const CSSSize& aNewSize,
ResizeEventFlag aResizeEventFlag) {
void GeckoMVMContext::Reflow(const CSSSize& aNewSize) {
MOZ_ASSERT(mPresShell);
ResizeReflowOptions reflowOptions = ResizeReflowOptions::NoOption;
if (aResizeEventFlag == ResizeEventFlag::Suppress) {
reflowOptions |= ResizeReflowOptions::SuppressResizeEvent;
}
RefPtr<PresShell> presShell = mPresShell;
presShell->ResizeReflowIgnoreOverride(
nsPresContext::CSSPixelsToAppUnits(aNewSize.width),
nsPresContext::CSSPixelsToAppUnits(aNewSize.height),
reflowOptions);
CSSPixel::ToAppUnits(aNewSize.width),
CSSPixel::ToAppUnits(aNewSize.height),
ResizeReflowOptions::NoOption);
}
} // namespace mozilla

View File

@ -54,8 +54,7 @@ class GeckoMVMContext : public MVMContext {
ResolutionChangeOrigin aOrigin) override;
void SetVisualViewportSize(const CSSSize& aSize) override;
void UpdateDisplayPortMargins() override;
MOZ_CAN_RUN_SCRIPT_BOUNDARY
void Reflow(const CSSSize& aNewSize, ResizeEventFlag) override;
MOZ_CAN_RUN_SCRIPT_BOUNDARY void Reflow(const CSSSize& aNewSize) override;
private:
RefPtr<dom::Document> mDocument;

View File

@ -59,11 +59,7 @@ class MVMContext {
virtual void SetVisualViewportSize(const CSSSize& aSize) = 0;
virtual void UpdateDisplayPortMargins() = 0;
enum class ResizeEventFlag {
IfNecessary, // resize events will be fired if necessary.
Suppress, // resize events will never be fired.
};
virtual void Reflow(const CSSSize& aNewSize, ResizeEventFlag) = 0;
virtual void Reflow(const CSSSize& aNewSize) = 0;
};
} // namespace mozilla

View File

@ -595,9 +595,7 @@ void MobileViewportManager::RefreshViewportSize(bool aForceAdjustResolution) {
RefPtr<MobileViewportManager> strongThis(this);
// Kick off a reflow.
mContext->Reflow(viewport,
mIsFirstPaint ? MVMContext::ResizeEventFlag::Suppress
: MVMContext::ResizeEventFlag::IfNecessary);
mContext->Reflow(viewport);
// We are going to fit the content to the display width if the initial-scale
// is not specied and if the content is still wider than the display width.

View File

@ -1899,9 +1899,8 @@ nsresult PresShell::ResizeReflowIgnoreOverride(nscoord aWidth, nscoord aHeight,
const bool initialized = mDidInitialize;
RefPtr<PresShell> kungFuDeathGrip(this);
auto postResizeEventIfNeeded = [this, initialized, aOptions]() {
if (initialized && !mIsDestroying && !mResizeEventPending &&
!(aOptions & ResizeReflowOptions::SuppressResizeEvent)) {
auto postResizeEventIfNeeded = [this, initialized]() {
if (initialized && !mIsDestroying && !mResizeEventPending) {
mResizeEventPending = true;
if (MOZ_LIKELY(!mDocument->GetBFCacheEntry())) {
mPresContext->RefreshDriver()->AddResizeEventFlushObserver(this);

View File

@ -36,17 +36,12 @@ enum class ResizeReflowOptions : uint32_t {
// the resulting BSize can be less than the given one, producing
// shrink-to-fit sizing in the block dimension
BSizeLimit = 1 << 0,
// suppress resize events even if the content size is changed due to the
// reflow. This flag is used for mobile since on mobile we need to do an
// additional reflow to zoom the content by the initial-scale or auto scaling
// and we don't want any resize events during the initial paint.
SuppressResizeEvent = 1 << 1,
// Invalidate layout, but don't reflow.
//
// TODO(emilio): Ideally this should just become the default, or we should
// unconditionally not reflow and rely on the caller to do so, having a
// separate API for shrink-to-fit.
SuppressReflow = 1 << 2,
SuppressReflow = 1 << 1,
};
MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(ResizeReflowOptions)