Bug 912700 - Audit all uses of FrameMetrics::mViewport and wrap it in getter/setters. r=botond
authorKartikaya Gupta <kgupta@mozilla.com>
Fri, 22 Aug 2014 23:18:56 -0400
changeset 201210 95a32b449dc56b8fc19937f20eb95e4af5cdce1c
parent 201209 bf2bf138571c10690583eb6e14d780adb1bbb205
child 201211 0ae7772c79778408cf10731476759487ff9c39aa
push id48106
push userkgupta@mozilla.com
push dateSat, 23 Aug 2014 03:19:05 +0000
treeherdermozilla-inbound@95a32b449dc5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs912700
milestone34.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 912700 - Audit all uses of FrameMetrics::mViewport and wrap it in getter/setters. r=botond
dom/ipc/TabChild.cpp
gfx/layers/FrameMetrics.h
gfx/layers/LayersLogging.cpp
gfx/layers/apz/src/AsyncPanZoomController.cpp
gfx/tests/gtest/TestAsyncPanZoomController.cpp
layout/base/nsDisplayList.cpp
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -156,17 +156,17 @@ NS_IMPL_CYCLE_COLLECTING_ADDREF(TabChild
 NS_IMPL_CYCLE_COLLECTING_RELEASE(TabChildBase)
 
 void
 TabChildBase::InitializeRootMetrics()
 {
   // Calculate a really simple resolution that we probably won't
   // be keeping, as well as putting the scroll offset back to
   // the top-left of the page.
-  mLastRootMetrics.mViewport = CSSRect(CSSPoint(), kDefaultViewportSize);
+  mLastRootMetrics.SetViewport(CSSRect(CSSPoint(), kDefaultViewportSize));
   mLastRootMetrics.mCompositionBounds = ParentLayerRect(
       ParentLayerPoint(),
       ParentLayerSize(ViewAs<ParentLayerPixel>(mInnerSize, PixelCastJustification::ScreenToParentLayerForRoot)));
   mLastRootMetrics.SetZoom(mLastRootMetrics.CalculateIntrinsicScale());
   mLastRootMetrics.mDevPixelsPerCSSPixel = WebWidget()->GetDefaultScale();
   // We use ScreenToLayerScale(1) below in order to turn the
   // async zoom amount into the gecko zoom amount.
   mLastRootMetrics.mCumulativeResolution =
@@ -255,17 +255,18 @@ TabChildBase::HandlePossibleViewportChan
   // that will just confuse future adjustments.
   if (!screenW || !screenH) {
     return false;
   }
 
   TABC_LOG("HandlePossibleViewportChange mOldViewportSize=%s viewport=%s\n",
     Stringify(mOldViewportSize).c_str(), Stringify(viewport).c_str());
   CSSSize oldBrowserSize = mOldViewportSize;
-  mLastRootMetrics.mViewport.SizeTo(viewport);
+  mLastRootMetrics.SetViewport(CSSRect(
+    mLastRootMetrics.GetViewport().TopLeft(), viewport));
   if (oldBrowserSize == CSSSize()) {
     oldBrowserSize = kDefaultViewportSize;
   }
   SetCSSViewport(viewport);
 
   // If this page has not been painted yet, then this must be getting run
   // because a meta-viewport element was added (via the DOMMetaAdded handler).
   // in this case, we should not do anything that forces a reflow (see bug
@@ -280,17 +281,17 @@ TabChildBase::HandlePossibleViewportChan
   }
 
   ScreenIntSize oldScreenSize = aOldScreenSize;
   if (oldScreenSize == ScreenIntSize()) {
     oldScreenSize = mInnerSize;
   }
 
   FrameMetrics metrics(mLastRootMetrics);
-  metrics.mViewport = CSSRect(CSSPoint(), viewport);
+  metrics.SetViewport(CSSRect(CSSPoint(), viewport));
   metrics.mCompositionBounds = ParentLayerRect(
       ParentLayerPoint(),
       ParentLayerSize(ViewAs<ParentLayerPixel>(mInnerSize, PixelCastJustification::ScreenToParentLayerForRoot)));
   metrics.SetRootCompositionSize(
       ScreenSize(mInnerSize) * ScreenToLayoutDeviceScale(1.0f) / metrics.mDevPixelsPerCSSPixel);
 
   // This change to the zoom accounts for all types of changes I can conceive:
   // 1. screen size changes, CSS viewport does not (pages with no meta viewport
@@ -477,19 +478,19 @@ TabChildBase::ProcessUpdateFrame(const F
     // The BrowserElementScrolling helper must know about these updated metrics
     // for other functions it performs, such as double tap handling.
     // Note, %f must not be used because it is locale specific!
     nsString data;
     data.AppendPrintf("{ \"x\" : %d", NS_lround(newMetrics.GetScrollOffset().x));
     data.AppendPrintf(", \"y\" : %d", NS_lround(newMetrics.GetScrollOffset().y));
     data.AppendLiteral(", \"viewport\" : ");
         data.AppendLiteral("{ \"width\" : ");
-        data.AppendFloat(newMetrics.mViewport.width);
+        data.AppendFloat(newMetrics.GetViewport().width);
         data.AppendLiteral(", \"height\" : ");
-        data.AppendFloat(newMetrics.mViewport.height);
+        data.AppendFloat(newMetrics.GetViewport().height);
         data.AppendLiteral(" }");
     data.AppendLiteral(", \"cssPageRect\" : ");
         data.AppendLiteral("{ \"x\" : ");
         data.AppendFloat(newMetrics.mScrollableRect.x);
         data.AppendLiteral(", \"y\" : ");
         data.AppendFloat(newMetrics.mScrollableRect.y);
         data.AppendLiteral(", \"width\" : ");
         data.AppendFloat(newMetrics.mScrollableRect.width);
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -71,17 +71,16 @@ public:
   static const ViewID NULL_SCROLL_ID;   // This container layer does not scroll.
   static const ViewID START_SCROLL_ID = 2;  // This is the ID that scrolling subframes
                                         // will begin at.
 
   FrameMetrics()
     : mCompositionBounds(0, 0, 0, 0)
     , mDisplayPort(0, 0, 0, 0)
     , mCriticalDisplayPort(0, 0, 0, 0)
-    , mViewport(0, 0, 0, 0)
     , mScrollableRect(0, 0, 0, 0)
     , mResolution(1)
     , mCumulativeResolution(1)
     , mTransformScale(1)
     , mDevPixelsPerCSSPixel(1)
     , mMayHaveTouchListeners(false)
     , mMayHaveTouchCaret(false)
     , mIsRoot(false)
@@ -90,16 +89,17 @@ public:
     , mScrollOffset(0, 0)
     , mZoom(1)
     , mUpdateScrollOffset(false)
     , mScrollGeneration(0)
     , mRootCompositionSize(0, 0)
     , mDisplayPortMargins(0, 0, 0, 0)
     , mUseDisplayPortMargins(false)
     , mPresShellId(-1)
+    , mViewport(0, 0, 0, 0)
   {}
 
   // Default copy ctor and operator= are fine
 
   bool operator==(const FrameMetrics& aOther) const
   {
     return mCompositionBounds.IsEqualEdges(aOther.mCompositionBounds) &&
            mRootCompositionSize == aOther.mRootCompositionSize &&
@@ -282,27 +282,16 @@ public:
   // If non-empty, the area of a frame's contents that is considered critical
   // to paint. Area outside of this area (i.e. area inside mDisplayPort, but
   // outside of mCriticalDisplayPort) is considered low-priority, and may be
   // painted with lower precision, or not painted at all.
   //
   // The same restrictions for mDisplayPort apply here.
   CSSRect mCriticalDisplayPort;
 
-  // The CSS viewport, which is the dimensions we're using to constrain the
-  // <html> element of this frame, relative to the top-left of the layer. Note
-  // that its offset is structured in such a way that it doesn't depend on the
-  // method layout uses to scroll content.
-  //
-  // This is mainly useful on the root layer, however nested iframes can have
-  // their own viewport, which will just be the size of the window of the
-  // iframe. For layers that don't correspond to a document, this metric is
-  // meaningless and invalid.
-  CSSRect mViewport;
-
   // The scrollable bounds of a frame. This is determined by reflow.
   // Ordinarily the x and y will be 0 and the width and height will be the
   // size of the element being scrolled. However for RTL pages or elements
   // the x value may be negative.
   //
   // This is relative to the document. It is in the same coordinate space as
   // |mScrollOffset|, but a different coordinate space than |mViewport| and
   // |mDisplayPort|. Note also that this coordinate system is understood by
@@ -443,16 +432,26 @@ public:
     return mPresShellId;
   }
 
   void SetPresShellId(uint32_t aPresShellId)
   {
     mPresShellId = aPresShellId;
   }
 
+  void SetViewport(const CSSRect& aViewport)
+  {
+    mViewport = aViewport;
+  }
+
+  const CSSRect& GetViewport() const
+  {
+    return mViewport;
+  }
+
 private:
   // New fields from now on should be made private and old fields should
   // be refactored to be private.
 
   // Whether or not this frame is for an element marked 'scrollgrab'.
   bool mHasScrollgrab;
 
   // A unique ID assigned to each scrollable frame.
@@ -494,16 +493,27 @@ private:
   // is drawn of the scrollable element.
   LayerMargin mDisplayPortMargins;
 
   // If this is true then we use the display port margins on this metrics,
   // otherwise use the display port rect.
   bool mUseDisplayPortMargins;
 
   uint32_t mPresShellId;
+
+  // The CSS viewport, which is the dimensions we're using to constrain the
+  // <html> element of this frame, relative to the top-left of the layer. Note
+  // that its offset is structured in such a way that it doesn't depend on the
+  // method layout uses to scroll content.
+  //
+  // This is mainly useful on the root layer, however nested iframes can have
+  // their own viewport, which will just be the size of the window of the
+  // iframe. For layers that don't correspond to a document, this metric is
+  // meaningless and invalid.
+  CSSRect mViewport;
 };
 
 /**
  * This class allows us to uniquely identify a scrollable layer. The
  * mLayersId identifies the layer tree (corresponding to a child process
  * and/or tab) that the scrollable layer belongs to. The mPresShellId
  * is a temporal identifier (corresponding to the document loaded that
  * contains the scrollable layer, which may change over time). The
--- a/gfx/layers/LayersLogging.cpp
+++ b/gfx/layers/LayersLogging.cpp
@@ -125,17 +125,17 @@ AppendToString(std::stringstream& aStrea
   AppendToString(aStream, m.mCriticalDisplayPort, " cdp=");
   if (!detailed) {
     AppendToString(aStream, m.GetScrollId(), " scrollId=");
     aStream << nsPrintfCString(" z=%.3f }", m.GetZoom().scale).get();
   } else {
     AppendToString(aStream, m.GetDisplayPortMargins(), " dpm=");
     aStream << nsPrintfCString(" um=%d", m.GetUseDisplayPortMargins()).get();
     AppendToString(aStream, m.GetRootCompositionSize(), " rcs=");
-    AppendToString(aStream, m.mViewport, " v=");
+    AppendToString(aStream, m.GetViewport(), " v=");
     aStream << nsPrintfCString(" z=(ld=%.3f r=%.3f cr=%.3f z=%.3f ts=%.3f)",
             m.mDevPixelsPerCSSPixel.scale, m.mResolution.scale,
             m.mCumulativeResolution.scale, m.GetZoom().scale,
             m.mTransformScale.scale).get();
     aStream << nsPrintfCString(" u=(%d %lu)",
             m.GetScrollOffsetUpdated(), m.GetScrollGeneration()).get();
     aStream << nsPrintfCString(" i=(%ld %lld) }",
             m.GetPresShellId(), m.GetScrollId()).get();
--- a/gfx/layers/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2097,18 +2097,18 @@ void AsyncPanZoomController::RequestCont
       fabsf(marginDelta.top) < EPSILON &&
       fabsf(marginDelta.right) < EPSILON &&
       fabsf(marginDelta.bottom) < EPSILON &&
       fabsf(mLastPaintRequestMetrics.GetScrollOffset().x -
             aFrameMetrics.GetScrollOffset().x) < EPSILON &&
       fabsf(mLastPaintRequestMetrics.GetScrollOffset().y -
             aFrameMetrics.GetScrollOffset().y) < EPSILON &&
       aFrameMetrics.GetZoom() == mLastPaintRequestMetrics.GetZoom() &&
-      fabsf(aFrameMetrics.mViewport.width - mLastPaintRequestMetrics.mViewport.width) < EPSILON &&
-      fabsf(aFrameMetrics.mViewport.height - mLastPaintRequestMetrics.mViewport.height) < EPSILON) {
+      fabsf(aFrameMetrics.GetViewport().width - mLastPaintRequestMetrics.GetViewport().width) < EPSILON &&
+      fabsf(aFrameMetrics.GetViewport().height - mLastPaintRequestMetrics.GetViewport().height) < EPSILON) {
     return;
   }
 
   SendAsyncScrollEvent();
   mPaintThrottler.PostTask(
     FROM_HERE,
     UniquePtr<CancelableTask>(NewRunnableMethod(this,
                       &AsyncPanZoomController::DispatchRepaintRequest,
@@ -2425,21 +2425,21 @@ void AsyncPanZoomController::NotifyLayer
   }
 
   mPaintThrottler.TaskComplete(GetFrameTime());
   bool needContentRepaint = false;
   if (FuzzyEqualsAdditive(aLayerMetrics.mCompositionBounds.width, mFrameMetrics.mCompositionBounds.width) &&
       FuzzyEqualsAdditive(aLayerMetrics.mCompositionBounds.height, mFrameMetrics.mCompositionBounds.height)) {
     // Remote content has sync'd up to the composition geometry
     // change, so we can accept the viewport it's calculated.
-    if (mFrameMetrics.mViewport.width != aLayerMetrics.mViewport.width ||
-        mFrameMetrics.mViewport.height != aLayerMetrics.mViewport.height) {
+    if (mFrameMetrics.GetViewport().width != aLayerMetrics.GetViewport().width ||
+        mFrameMetrics.GetViewport().height != aLayerMetrics.GetViewport().height) {
       needContentRepaint = true;
     }
-    mFrameMetrics.mViewport = aLayerMetrics.mViewport;
+    mFrameMetrics.SetViewport(aLayerMetrics.GetViewport());
   }
 
   // If the layers update was not triggered by our own repaint request, then
   // we want to take the new scroll offset. Check the scroll generation as well
   // to filter duplicate calls to NotifyLayersUpdated with the same scroll offset
   // update message.
   bool scrollOffsetUpdated = aLayerMetrics.GetScrollOffsetUpdated()
         && (aLayerMetrics.GetScrollGeneration() != mFrameMetrics.GetScrollGeneration());
--- a/gfx/tests/gtest/TestAsyncPanZoomController.cpp
+++ b/gfx/tests/gtest/TestAsyncPanZoomController.cpp
@@ -175,17 +175,16 @@ static FrameMetrics
 TestFrameMetrics()
 {
   FrameMetrics fm;
 
   fm.mDisplayPort = CSSRect(0, 0, 10, 10);
   fm.mCompositionBounds = ParentLayerRect(0, 0, 10, 10);
   fm.mCriticalDisplayPort = CSSRect(0, 0, 10, 10);
   fm.mScrollableRect = CSSRect(0, 0, 100, 100);
-  fm.mViewport = CSSRect(0, 0, 10, 10);
 
   return fm;
 }
 
 class APZCBasicTester : public ::testing::Test {
 public:
   APZCBasicTester(AsyncPanZoomController::GestureBehavior aGestureBehavior = AsyncPanZoomController::DEFAULT_GESTURES)
     : mGestureBehavior(aGestureBehavior)
@@ -493,17 +492,16 @@ public:
     : APZCBasicTester(aGestureBehavior)
   {
   }
 
 protected:
   FrameMetrics GetPinchableFrameMetrics()
   {
     FrameMetrics fm;
-    fm.mViewport = CSSRect(0, 0, 980, 480);
     fm.mCompositionBounds = ParentLayerRect(200, 200, 100, 200);
     fm.mScrollableRect = CSSRect(0, 0, 980, 1000);
     fm.SetScrollOffset(CSSPoint(300, 300));
     fm.SetZoom(CSSToScreenScale(2.0));
     // the visible area of the document in CSS pixels is x=300 y=300 w=50 h=100
     return fm;
   }
 
@@ -635,17 +633,16 @@ TEST_F(APZCPinchGestureDetectorTester, P
   EXPECT_EQ(originalMetrics.GetScrollOffset().y, fm.GetScrollOffset().y);
 
   apzc->AssertStateIsReset();
 }
 
 TEST_F(APZCBasicTester, Overzoom) {
   // the visible area of the document in CSS pixels is x=10 y=0 w=100 h=100
   FrameMetrics fm;
-  fm.mViewport = CSSRect(0, 0, 100, 100);
   fm.mCompositionBounds = ParentLayerRect(0, 0, 100, 100);
   fm.mScrollableRect = CSSRect(0, 0, 125, 150);
   fm.SetScrollOffset(CSSPoint(10, 0));
   fm.SetZoom(CSSToScreenScale(1.0));
   apzc->SetFrameMetrics(fm);
 
   MakeApzcZoomable();
 
@@ -705,17 +702,16 @@ TEST_F(APZCBasicTester, ComplexTransform
 
   nsTArray<nsRefPtr<Layer> > layers;
   nsRefPtr<LayerManager> lm;
   nsRefPtr<Layer> root = CreateLayerTree(layerTreeSyntax, layerVisibleRegion, transforms, lm, layers);
 
   FrameMetrics metrics;
   metrics.mCompositionBounds = ParentLayerRect(0, 0, 24, 24);
   metrics.mDisplayPort = CSSRect(-1, -1, 6, 6);
-  metrics.mViewport = CSSRect(0, 0, 4, 4);
   metrics.SetScrollOffset(CSSPoint(10, 10));
   metrics.mScrollableRect = CSSRect(0, 0, 50, 50);
   metrics.mCumulativeResolution = LayoutDeviceToLayerScale(2);
   metrics.mResolution = ParentLayerToLayerScale(2);
   metrics.SetZoom(CSSToScreenScale(6));
   metrics.mDevPixelsPerCSSPixel = CSSToLayoutDeviceScale(3);
   metrics.SetScrollId(FrameMetrics::START_SCROLL_ID);
 
--- a/layout/base/nsDisplayList.cpp
+++ b/layout/base/nsDisplayList.cpp
@@ -654,17 +654,17 @@ static void RecordFrameMetrics(nsIFrame*
                                bool aIsRoot,
                                const ContainerLayerParameters& aContainerParameters) {
   nsPresContext* presContext = aForFrame->PresContext();
   int32_t auPerDevPixel = presContext->AppUnitsPerDevPixel();
   LayoutDeviceToLayerScale resolution(aContainerParameters.mXScale, aContainerParameters.mYScale);
 
   nsIPresShell* presShell = presContext->GetPresShell();
   FrameMetrics metrics;
-  metrics.mViewport = CSSRect::FromAppUnits(aViewport);
+  metrics.SetViewport(CSSRect::FromAppUnits(aViewport));
 
   ViewID scrollId = FrameMetrics::NULL_SCROLL_ID;
   nsIContent* content = aScrollFrame ? aScrollFrame->GetContent() : nullptr;
   if (content) {
     if (!aForceNullScrollId) {
       scrollId = nsLayoutUtils::FindOrCreateIDFor(content);
     }
     nsRect dp;