Bug 1062411 - Make FrameMetrics::mContentDescription an nsCString so there is no limit to its length. r=BenWa
authorBotond Ballo <botond@mozilla.com>
Wed, 03 Sep 2014 12:54:26 -0400
changeset 203476 9fe5fe620802dc4eed07c30a24de21300c112f91
parent 203475 338372590fc44d51ac28d183ad46afc674caed39
child 203477 2771520aa1b942d84e9f7c3b8571be1776e3b3a1
push id27428
push usercbook@mozilla.com
push dateThu, 04 Sep 2014 13:00:04 +0000
treeherdermozilla-central@7bfd030e8fc8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersBenWa
bugs1062411
milestone35.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 1062411 - Make FrameMetrics::mContentDescription an nsCString so there is no limit to its length. r=BenWa
gfx/layers/FrameMetrics.h
gfx/layers/apz/src/AsyncPanZoomController.cpp
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -55,20 +55,16 @@ typedef gfx::ScaleFactor<ParentLayerPixe
 
 namespace layers {
 
 /**
  * The viewport and displayport metrics for the painted frame at the
  * time of a layer-tree transaction.  These metrics are especially
  * useful for shadow layers, because the metrics values are updated
  * atomically with new pixels.
- *
- * Note that the FrameMetrics struct is sometimes stored in shared
- * memory and shared across processes, so it should be a "Plain Old
- * Data (POD)" type with no members that use dynamic memory.
  */
 struct FrameMetrics {
   friend struct IPC::ParamTraits<mozilla::layers::FrameMetrics>;
 public:
   // We use IDs to identify frames across processes.
   typedef uint64_t ViewID;
   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
@@ -96,17 +92,16 @@ public:
     , mScrollGeneration(0)
     , mRootCompositionSize(0, 0)
     , mDisplayPortMargins(0, 0, 0, 0)
     , mUseDisplayPortMargins(false)
     , mPresShellId(-1)
     , mViewport(0, 0, 0, 0)
     , mBackgroundColor(0, 0, 0, 0)
   {
-    mContentDescription[0] = '\0';
   }
 
   // Default copy ctor and operator= are fine
 
   bool operator==(const FrameMetrics& aOther) const
   {
     return mCompositionBounds.IsEqualEdges(aOther.mCompositionBounds) &&
            mRootCompositionSize == aOther.mRootCompositionSize &&
@@ -123,18 +118,17 @@ public:
            mMayHaveTouchCaret == aOther.mMayHaveTouchCaret &&
            mPresShellId == aOther.mPresShellId &&
            mIsRoot == aOther.mIsRoot &&
            mScrollId == aOther.mScrollId &&
            mScrollParentId == aOther.mScrollParentId &&
            mScrollOffset == aOther.mScrollOffset &&
            mHasScrollgrab == aOther.mHasScrollgrab &&
            mUpdateScrollOffset == aOther.mUpdateScrollOffset &&
-           mBackgroundColor == aOther.mBackgroundColor &&
-           !strcmp(mContentDescription, aOther.mContentDescription);
+           mBackgroundColor == aOther.mBackgroundColor;
   }
   bool operator!=(const FrameMetrics& aOther) const
   {
     return !operator==(aOther);
   }
 
   bool IsDefault() const
   {
@@ -242,16 +236,26 @@ public:
   }
 
   void CopyScrollInfoFrom(const FrameMetrics& aOther)
   {
     mScrollOffset = aOther.mScrollOffset;
     mScrollGeneration = aOther.mScrollGeneration;
   }
 
+  // Make a copy of this FrameMetrics object which does not have any pointers
+  // to heap-allocated memory (i.e. is Plain Old Data, or 'POD'), and is
+  // therefore safe to be placed into shared memory.
+  FrameMetrics MakePODObject() const
+  {
+    FrameMetrics copy = *this;
+    copy.mContentDescription.Truncate();
+    return copy;
+  }
+
   // ---------------------------------------------------------------------------
   // The following metrics are all in widget space/device pixels.
   //
 
   // This is the area within the widget that we're compositing to. It is relative
   // to the layer tree origin.
   //
   // This is useful because, on mobile, the viewport and composition dimensions
@@ -479,26 +483,24 @@ public:
     return mBackgroundColor;
   }
 
   void SetBackgroundColor(const gfxRGBA& aBackgroundColor)
   {
     mBackgroundColor = aBackgroundColor;
   }
 
-  nsCString GetContentDescription() const
+  const nsCString& GetContentDescription() const
   {
-    return nsCString(mContentDescription);
+    return mContentDescription;
   }
 
   void SetContentDescription(const nsCString& aContentDescription)
   {
-    strncpy(mContentDescription, aContentDescription.get(),
-            sizeof(mContentDescription));
-    mContentDescription[sizeof(mContentDescription) - 1] = 0;
+    mContentDescription = aContentDescription;
   }
 
 private:
   // New fields from now on should be made private and old fields should
   // be refactored to be private.
 
   // Whether or not this is the root scroll frame for the root content document.
   bool mIsRoot;
@@ -564,19 +566,19 @@ private:
   // iframe. For layers that don't correspond to a document, this metric is
   // meaningless and invalid.
   CSSRect mViewport;
 
   // The background color to use when overscrolling.
   gfxRGBA mBackgroundColor;
 
   // A description of the content element corresponding to this frame.
-  // This is empty unless this is a scrollable ContainerLayer and the
+  // This is empty unless this is a scrollable layer and the
   // apz.printtree pref is turned on.
-  char mContentDescription[20];
+  nsCString mContentDescription;
 };
 
 /**
  * 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/apz/src/AsyncPanZoomController.cpp
+++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp
@@ -2863,17 +2863,17 @@ void AsyncPanZoomController::UpdateShare
 {
   mMonitor.AssertCurrentThreadIn();
 
   FrameMetrics* frame = mSharedFrameMetricsBuffer ?
       static_cast<FrameMetrics*>(mSharedFrameMetricsBuffer->memory()) : nullptr;
 
   if (frame && mSharedLock && gfxPrefs::UseProgressiveTilePainting()) {
     mSharedLock->Lock();
-    *frame = mFrameMetrics;
+    *frame = mFrameMetrics.MakePODObject();
     mSharedLock->Unlock();
   }
 }
 
 void AsyncPanZoomController::ShareCompositorFrameMetrics() {
 
   PCompositorParent* compositor = GetSharedFrameMetricsCompositor();