Bug 1382593: Clean a bit nsImageMap. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 03 Jul 2017 20:35:14 +0200
changeset 613383 3de77c4c75f80bc87029a570ae077fbe99e46986
parent 613152 9c8c36344fee1097adf411124ec637ecc2942be8
child 638652 0c1eb4ca88e56b2cf79775d7d66427d98f73727f
push id69772
push userbmo:emilio+bugs@crisal.io
push dateFri, 21 Jul 2017 20:49:00 +0000
reviewersheycam
bugs1382593
milestone56.0a1
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;
 };