No Issue - Modernize nsImageFrame/nsImageMap

Primarily flow control improvements, and removing unused `nsresults`.

layout/generic/nsImageFrame.cpp

@@ -175,18 +175,19 @@ nsImageFrame::AccessibleType()
 void
 nsImageFrame::DisconnectMap()
 {
-  if (mImageMap) {
+  if (!mImageMap) {
+    return;
+  }
+  
   mImageMap->Destroy();
   mImageMap = nullptr;
 
 #ifdef ACCESSIBILITY
-  nsAccessibilityService* accService = GetAccService();
+  if (nsAccessibilityService* accService = GetAccService()) {
-  if (accService) {
     accService->RecreateAccessible(PresContext()->PresShell(), mContent);
   }
 #endif
 }
-}
 
 void
 nsImageFrame::DestroyFrom(nsIFrame* aDestructRoot)
@@ -1718,8 +1719,7 @@ nsImageFrame::PaintImage(nsRenderingContext& aRenderingContext, nsPoint aPt,
       nsLayoutUtils::GetSamplingFilterForFrame(this), dest, aDirtyRect,
       nullptr, flags, &anchorPoint);
 
-  nsImageMap* map = GetImageMap();
+  if (nsImageMap* map = GetImageMap()) {
-  if (map) {
     gfxPoint devPixelOffset =
       nsLayoutUtils::PointToGfxPoint(dest.TopLeft(),
                                      PresContext()->AppUnitsPerDevPixel());
@@ -1886,8 +1886,7 @@ nsImageMap*
 nsImageFrame::GetImageMap()
 {
   if (!mImageMap) {
-    nsIContent* map = GetMapElement();
+    if (nsIContent* map = GetMapElement()) {
-    if (map) {
       mImageMap = new nsImageMap();
       mImageMap->Init(this, map);
     }
@@ -1975,9 +1974,7 @@ nsImageFrame::GetContentForEvent(WidgetEvent* aEvent,
     return NS_OK;
   }
 
-  nsImageMap* map = GetImageMap();
+  if (nsImageMap* map = GetImageMap()) {
-
-  if (nullptr != map) {
     nsIntPoint p;
     TranslateEventCoords(
       nsLayoutUtils::GetEventCoordinatesRelativeTo(aEvent, this), p);
@@ -2006,7 +2003,7 @@ nsImageFrame::HandleEvent(nsPresContext* aPresContext,
       aEvent->mMessage == eMouseMove) {
     nsImageMap* map = GetImageMap();
     bool isServerMap = IsServerImageMap();
-    if ((nullptr != map) || isServerMap) {
+    if (map || isServerMap) {
       nsIntPoint p;
       TranslateEventCoords(
         nsLayoutUtils::GetEventCoordinatesRelativeTo(aEvent, this), p);
@@ -2015,7 +2012,7 @@ nsImageFrame::HandleEvent(nsPresContext* aPresContext,
       // 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!)
-      if (nullptr != map) {
+      if (map) {
         inside = !!map->GetArea(p.x, p.y);
       }
 
@@ -2063,8 +2060,7 @@ nsresult
 nsImageFrame::GetCursor(const nsPoint& aPoint,
                         nsIFrame::Cursor& aCursor)
 {
-  nsImageMap* map = GetImageMap();
+  if (nsImageMap* map = GetImageMap()) {
-  if (nullptr != map) {
     nsIntPoint p;
     TranslateEventCoords(aPoint, p);
     nsCOMPtr<nsIContent> area = map->GetArea(p.x, p.y);

layout/generic/nsImageMap.cpp

@@ -715,9 +715,7 @@ nsImageMap::GetBoundsForAreaContent(nsIContent *aContent,
 void
 nsImageMap::FreeAreas()
 {
-  uint32_t i, n = mAreas.Length();
+  for (auto* area : mAreas) {
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
     if (area->mArea->IsInUncomposedDoc()) {
       NS_ASSERTION(area->mArea->GetPrimaryFrame() == mImageFrame,
                    "Unexpected primary frame");
@@ -731,30 +729,28 @@ nsImageMap::FreeAreas()
                                            false);
     delete area;
   }
+
   mAreas.Clear();
 }
 
-nsresult
+void
 nsImageMap::Init(nsImageFrame* aImageFrame, nsIContent* aMap)
 {
-  NS_PRECONDITION(aMap, "null ptr");
+  MOZ_ASSERT(aMap);
-  if (!aMap) {
+  MOZ_ASSERT(aImageFrame);
-    return NS_ERROR_NULL_POINTER;
-  }
-  mImageFrame = 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)
 {
-  nsresult rv = NS_OK;
   uint32_t n = aParent->GetChildCount();
 
   // Look for <area> elements.
@@ -762,8 +758,7 @@ nsImageMap::SearchForAreas(nsIContent* aParent)
        child;
        child = child->GetNextSibling()) {
     if (auto* area = static_cast<HTMLAreaElement*>(HTMLAreaElement::FromContent(child))) {
-      rv = AddArea(area);
+      AddArea(area);
-      NS_ENSURE_SUCCESS(rv, rv);
 
       // Continue to next child. This stops mConsiderWholeSubtree from
       // getting set. It also makes us ignore children of <area>s which
@@ -774,35 +769,28 @@ nsImageMap::SearchForAreas(nsIContent* aParent)
 
     if (child->IsElement()) {
       mConsiderWholeSubtree = true;
-      rv = SearchForAreas(child);
+      SearchForAreas(child);
-      NS_ENSURE_SUCCESS(rv, rv);
+    }
   }
 }
 
-  return NS_OK;
+void
-}
-
-nsresult
 nsImageMap::UpdateAreas()
 {
   // Get rid of old area data
   FreeAreas();
 
   mConsiderWholeSubtree = false;
-  nsresult rv = SearchForAreas(mMap);
+  SearchForAreas(mMap);
 
 #ifdef ACCESSIBILITY
-  if (NS_SUCCEEDED(rv)) {
+  if (nsAccessibilityService* accService = GetAccService()) {
-    nsAccessibilityService* accService = GetAccService();
-    if (accService) {
     accService->UpdateImageMap(mImageFrame);
   }
-  }
 #endif
-  return rv;
 }
 
-nsresult
+void
 nsImageMap::AddArea(HTMLAreaElement* aArea)
 {
   static nsIContent::AttrValuesArray strings[] =
@@ -834,17 +822,13 @@ nsImageMap::AddArea(HTMLAreaElement* 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,
+  aArea->AddSystemEventListener(NS_LITERAL_STRING("focus"), this, false, false);
-                                false);
+  aArea->AddSystemEventListener(NS_LITERAL_STRING("blur"), 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,
@@ -857,16 +841,13 @@ nsImageMap::AddArea(HTMLAreaElement* aArea)
   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 (auto* area : mAreas) {
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
     if (area->IsInside(aX, aY)) {
       return area->mArea;
     }
@@ -1000,7 +981,7 @@ nsImageMap::HandleEvent(nsIDOMEvent* aEvent)
 }
 
 void
-nsImageMap::Destroy(void)
+nsImageMap::Destroy()
 {
   FreeAreas();
   mImageFrame = nullptr;

layout/generic/nsImageMap.h

@@ -37,7 +37,7 @@ class nsImageMap final : public nsStubMutationObserver,
 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
@@ -86,10 +86,10 @@ protected:
 
   void FreeAreas();
 
-  nsresult UpdateAreas();
+  void UpdateAreas();
-  nsresult SearchForAreas(nsIContent* aParent);
+  void SearchForAreas(nsIContent* aParent);
 
-  nsresult AddArea(mozilla::dom::HTMLAreaElement* aArea);
+  void AddArea(mozilla::dom::HTMLAreaElement* aArea);
  
   void MaybeUpdateAreas(nsIContent *aContent);