Bug 1382593: Clean a bit nsImageMap. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 03 Jul 2017 20:35:14 +0200
changeset 419007 5a67359d51b46c42083f0188510d9ef608823947
parent 419006 cc9fbf7eabe7b007483b0b5f442f3e2fac1f159d
child 419008 b19a95c5c5b758ac90d50be3bd137dd3b2c64d75
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1382593
milestone56.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 1382593: Clean a bit nsImageMap. r=heycam MozReview-Commit-ID: Htm2OoExJ3T
layout/generic/nsImageFrame.cpp
layout/generic/nsImageMap.cpp
layout/generic/nsImageMap.h
--- a/layout/generic/nsImageFrame.cpp
+++ b/layout/generic/nsImageFrame.cpp
@@ -173,27 +173,28 @@ nsImageFrame::AccessibleType()
 
   return a11y::eImageType;
 }
 #endif
 
 void
 nsImageFrame::DisconnectMap()
 {
-  if (mImageMap) {
-    mImageMap->Destroy();
-    mImageMap = nullptr;
+  if (!mImageMap) {
+    return;
+  }
+
+  mImageMap->Destroy();
+  mImageMap = nullptr;
 
 #ifdef ACCESSIBILITY
-  nsAccessibilityService* accService = GetAccService();
-  if (accService) {
+  if (nsAccessibilityService* accService = GetAccService()) {
     accService->RecreateAccessible(PresContext()->PresShell(), mContent);
   }
 #endif
-  }
 }
 
 void
 nsImageFrame::DestroyFrom(nsIFrame* aDestructRoot)
 {
   if (mReflowCallbackPosted) {
     PresContext()->PresShell()->CancelReflowCallback(this);
     mReflowCallbackPosted = false;
@@ -1750,18 +1751,17 @@ nsImageFrame::PaintImage(gfxContext& aRe
   SVGImageContext::MaybeStoreContextPaint(svgContext, this, aImage);
 
   DrawResult result =
     nsLayoutUtils::DrawSingleImage(aRenderingContext,
       PresContext(), aImage,
       nsLayoutUtils::GetSamplingFilterForFrame(this), dest, aDirtyRect,
       svgContext, flags, &anchorPoint);
 
-  nsImageMap* map = GetImageMap();
-  if (map) {
+  if (nsImageMap* map = GetImageMap()) {
     gfxPoint devPixelOffset =
       nsLayoutUtils::PointToGfxPoint(dest.TopLeft(),
                                      PresContext()->AppUnitsPerDevPixel());
     AutoRestoreTransform autoRestoreTransform(drawTarget);
     drawTarget->SetTransform(
       drawTarget->GetTransform().PreTranslate(ToPoint(devPixelOffset)));
 
     // solid white stroke:
@@ -1924,18 +1924,17 @@ nsImageFrame::ShouldDisplaySelection()
 #endif
   return true;
 }
 
 nsImageMap*
 nsImageFrame::GetImageMap()
 {
   if (!mImageMap) {
-    nsIContent* map = GetMapElement();
-    if (map) {
+    if (nsIContent* map = GetMapElement()) {
       mImageMap = new nsImageMap();
       mImageMap->Init(this, map);
     }
   }
 
   return mImageMap;
 }
 
@@ -2013,19 +2012,17 @@ nsImageFrame::GetContentForEvent(WidgetE
     aEvent->HasMouseEventMessage() ? nsIPresShell::GetCapturingContent() :
                                      nullptr;
   if (capturingContent && capturingContent->GetPrimaryFrame() == this) {
     *aContent = capturingContent;
     NS_IF_ADDREF(*aContent);
     return NS_OK;
   }
 
-  nsImageMap* map = GetImageMap();
-
-  if (nullptr != map) {
+  if (nsImageMap* map = GetImageMap()) {
     nsIntPoint p;
     TranslateEventCoords(
       nsLayoutUtils::GetEventCoordinatesRelativeTo(aEvent, this), p);
     nsCOMPtr<nsIContent> area = map->GetArea(p.x, p.y);
     if (area) {
       area.forget(aContent);
       return NS_OK;
     }
@@ -2044,17 +2041,17 @@ nsImageFrame::HandleEvent(nsPresContext*
 {
   NS_ENSURE_ARG_POINTER(aEventStatus);
 
   if ((aEvent->mMessage == eMouseClick &&
        aEvent->AsMouseEvent()->button == WidgetMouseEvent::eLeftButton) ||
       aEvent->mMessage == eMouseMove) {
     nsImageMap* map = GetImageMap();
     bool isServerMap = IsServerImageMap();
-    if ((nullptr != map) || isServerMap) {
+    if (map || isServerMap) {
       nsIntPoint p;
       TranslateEventCoords(
         nsLayoutUtils::GetEventCoordinatesRelativeTo(aEvent, this), p);
       bool inside = false;
       // Even though client-side image map triggering happens
       // through content, we need to make sure we're not inside
       // (in case we deal with a case of both client-side and
       // sever-side on the same image - it happens!)
@@ -2101,18 +2098,17 @@ nsImageFrame::HandleEvent(nsPresContext*
 
   return nsAtomicContainerFrame::HandleEvent(aPresContext, aEvent, aEventStatus);
 }
 
 nsresult
 nsImageFrame::GetCursor(const nsPoint& aPoint,
                         nsIFrame::Cursor& aCursor)
 {
-  nsImageMap* map = GetImageMap();
-  if (nullptr != map) {
+  if (nsImageMap* map = GetImageMap()) {
     nsIntPoint p;
     TranslateEventCoords(aPoint, p);
     nsCOMPtr<nsIContent> area = map->GetArea(p.x, p.y);
     if (area) {
       // Use the cursor from the style of the *area* element.
       // XXX Using the image as the parent style context isn't
       // technically correct, but it's probably the right thing to do
       // here, since it means that areas on which the cursor isn't
--- a/layout/generic/nsImageMap.cpp
+++ b/layout/generic/nsImageMap.cpp
@@ -708,118 +708,104 @@ nsImageMap::GetBoundsForAreaContent(nsIC
     }
   }
   return NS_ERROR_FAILURE;
 }
 
 void
 nsImageMap::FreeAreas()
 {
-  uint32_t i, n = mAreas.Length();
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
+  for (auto* area : mAreas) {
     if (area->mArea->IsInUncomposedDoc()) {
       NS_ASSERTION(area->mArea->GetPrimaryFrame() == mImageFrame,
                    "Unexpected primary frame");
 
       area->mArea->SetPrimaryFrame(nullptr);
     }
 
     area->mArea->RemoveSystemEventListener(NS_LITERAL_STRING("focus"), this,
                                            false);
     area->mArea->RemoveSystemEventListener(NS_LITERAL_STRING("blur"), this,
                                            false);
     delete area;
   }
+
   mAreas.Clear();
 }
 
-nsresult
+void
 nsImageMap::Init(nsImageFrame* aImageFrame, nsIContent* aMap)
 {
-  NS_PRECONDITION(aMap, "null ptr");
-  if (!aMap) {
-    return NS_ERROR_NULL_POINTER;
-  }
+  MOZ_ASSERT(aMap);
+  MOZ_ASSERT(aImageFrame);
+
   mImageFrame = aImageFrame;
-
   mMap = aMap;
   mMap->AddMutationObserver(this);
 
   // "Compile" the areas in the map into faster access versions
-  return UpdateAreas();
+  UpdateAreas();
 }
 
-
-nsresult
+void
 nsImageMap::SearchForAreas(nsIContent* aParent, bool& aFoundArea,
                            bool& aFoundAnchor)
 {
-  nsresult rv = NS_OK;
   uint32_t i, n = aParent->GetChildCount();
 
   // Look for <area> or <a> elements. We'll use whichever type we find first.
   for (i = 0; i < n; i++) {
     nsIContent *child = aParent->GetChildAt(i);
 
     // If we haven't determined that the map element contains an
     // <a> element yet, then look for <area>.
     if (!aFoundAnchor && child->IsHTMLElement(nsGkAtoms::area)) {
       aFoundArea = true;
-      rv = AddArea(child);
-      NS_ENSURE_SUCCESS(rv, rv);
+      AddArea(child);
 
       // Continue to next child. This stops mContainsBlockContents from
       // getting set. It also makes us ignore children of <area>s which
       // is consistent with how we react to dynamic insertion of such
       // children.
       continue;
     }
 
     // If we haven't determined that the map element contains an
     // <area> element yet, then look for <a>.
     if (!aFoundArea && child->IsHTMLElement(nsGkAtoms::a)) {
       aFoundAnchor = true;
-      rv = AddArea(child);
-      NS_ENSURE_SUCCESS(rv, rv);
+      AddArea(child);
     }
 
     if (child->IsElement()) {
       mContainsBlockContents = true;
-      rv = SearchForAreas(child, aFoundArea, aFoundAnchor);
-      NS_ENSURE_SUCCESS(rv, rv);
+      SearchForAreas(child, aFoundArea, aFoundAnchor);
     }
   }
-
-  return NS_OK;
 }
 
-nsresult
+void
 nsImageMap::UpdateAreas()
 {
   // Get rid of old area data
   FreeAreas();
 
   bool foundArea = false;
   bool foundAnchor = false;
   mContainsBlockContents = false;
 
-  nsresult rv = SearchForAreas(mMap, foundArea, foundAnchor);
+  SearchForAreas(mMap, foundArea, foundAnchor);
 #ifdef ACCESSIBILITY
-  if (NS_SUCCEEDED(rv)) {
-    nsAccessibilityService* accService = GetAccService();
-    if (accService) {
-      accService->UpdateImageMap(mImageFrame);
-    }
+  if (nsAccessibilityService* accService = GetAccService()) {
+    accService->UpdateImageMap(mImageFrame);
   }
 #endif
-  return rv;
 }
 
-nsresult
+void
 nsImageMap::AddArea(nsIContent* aArea)
 {
   static nsIContent::AttrValuesArray strings[] =
     {&nsGkAtoms::rect, &nsGkAtoms::rectangle,
      &nsGkAtoms::circle, &nsGkAtoms::circ,
      &nsGkAtoms::_default,
      &nsGkAtoms::poly, &nsGkAtoms::polygon,
      nullptr};
@@ -841,49 +827,42 @@ nsImageMap::AddArea(nsIContent* aArea)
     area = new DefaultArea(aArea);
     break;
   case 5:
   case 6:
     area = new PolyArea(aArea);
     break;
   default:
     area = nullptr;
-    NS_NOTREACHED("FindAttrValueIn returned an unexpected value.");
+    MOZ_ASSERT_UNREACHABLE("FindAttrValueIn returned an unexpected value.");
     break;
   }
-  if (!area)
-    return NS_ERROR_OUT_OF_MEMORY;
 
   //Add focus listener to track area focus changes
-  aArea->AddSystemEventListener(NS_LITERAL_STRING("focus"), this, false,
-                                false);
-  aArea->AddSystemEventListener(NS_LITERAL_STRING("blur"), this, false,
-                                false);
+  aArea->AddSystemEventListener(NS_LITERAL_STRING("focus"), this, false, false);
+  aArea->AddSystemEventListener(NS_LITERAL_STRING("blur"), this, false, false);
 
   // This is a nasty hack.  It needs to go away: see bug 135040.  Once this is
   // removed, the code added to RestyleManager::RestyleElement,
   // nsCSSFrameConstructor::ContentRemoved (both hacks there), and
   // RestyleManager::ProcessRestyledFrames to work around this issue can
   // be removed.
   aArea->SetPrimaryFrame(mImageFrame);
 
   nsAutoString coords;
   aArea->GetAttr(kNameSpaceID_None, nsGkAtoms::coords, coords);
   area->ParseCoords(coords);
   mAreas.AppendElement(area);
-  return NS_OK;
 }
 
 nsIContent*
 nsImageMap::GetArea(nscoord aX, nscoord aY) const
 {
   NS_ASSERTION(mMap, "Not initialized");
-  uint32_t i, n = mAreas.Length();
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
+  for (auto* area : mAreas) {
     if (area->IsInside(aX, aY)) {
       return area->mArea;
     }
   }
 
   return nullptr;
 }
 
@@ -1007,14 +986,14 @@ nsImageMap::HandleEvent(nsIDOMEvent* aEv
       }
       break;
     }
   }
   return NS_OK;
 }
 
 void
-nsImageMap::Destroy(void)
+nsImageMap::Destroy()
 {
   FreeAreas();
   mImageFrame = nullptr;
   mMap->RemoveMutationObserver(this);
 }
--- a/layout/generic/nsImageMap.h
+++ b/layout/generic/nsImageMap.h
@@ -26,17 +26,17 @@ class nsImageMap final : public nsStubMu
 {
   typedef mozilla::gfx::DrawTarget DrawTarget;
   typedef mozilla::gfx::ColorPattern ColorPattern;
   typedef mozilla::gfx::StrokeOptions StrokeOptions;
 
 public:
   nsImageMap();
 
-  nsresult Init(nsImageFrame* aImageFrame, nsIContent* aMap);
+  void Init(nsImageFrame* aImageFrame, nsIContent* aMap);
 
   /**
    * Return the first area element (in content order) for the given aX,aY pixel
    * coordinate or nullptr if the coordinate is outside all areas.
    */
   nsIContent* GetArea(nscoord aX, nscoord aY) const;
 
   /**
@@ -75,21 +75,22 @@ public:
   nsresult GetBoundsForAreaContent(nsIContent *aContent,
                                    nsRect& aBounds);
 
 protected:
   virtual ~nsImageMap();
 
   void FreeAreas();
 
-  nsresult UpdateAreas();
-  nsresult SearchForAreas(nsIContent* aParent, bool& aFoundArea,
-                          bool& aFoundAnchor);
+  void UpdateAreas();
+  void SearchForAreas(nsIContent* aParent,
+                      bool& aFoundArea,
+                      bool& aFoundAnchor);
 
-  nsresult AddArea(nsIContent* aArea);
+  void AddArea(nsIContent* aArea);
 
   void MaybeUpdateAreas(nsIContent *aContent);
 
   nsImageFrame* mImageFrame;  // the frame that owns us
   nsCOMPtr<nsIContent> mMap;
   AutoTArray<Area*, 8> mAreas; // almost always has some entries
   bool mContainsBlockContents;
 };