Bug 1424952: Modernize a bit more nsImageMap, and make removals a bit more incremental. r=dholbert
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 15 Dec 2017 17:46:04 +0100
changeset 396535 3838a6073eb7ffcf9a5cb053b80964ed548e895e
parent 396534 96ba7b1e3254efb869040f1ba98d9f586df6ff4c
child 396536 34839f53008ffdba99c8c8284bf60aef48d6ca95
push id33097
push useraciure@mozilla.com
push dateFri, 15 Dec 2017 21:52:24 +0000
treeherdermozilla-central@4398768baa23 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1424952
milestone59.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 1424952: Modernize a bit more nsImageMap, and make removals a bit more incremental. r=dholbert Summary: MozReview-Commit-ID: 4iXJtLKEwY7 Reviewers: dholbert Reviewed By: dholbert Bug #: 1424952 Differential Revision: https://phabricator.services.mozilla.com/D336 MozReview-Commit-ID: 3FANAhS06Z5
layout/generic/nsImageMap.cpp
layout/generic/nsImageMap.h
--- a/layout/generic/nsImageMap.cpp
+++ b/layout/generic/nsImageMap.cpp
@@ -690,50 +690,50 @@ nsImageMap::~nsImageMap()
   NS_ASSERTION(mAreas.Length() == 0, "Destroy was not called");
 }
 
 NS_IMPL_ISUPPORTS(nsImageMap,
                   nsIMutationObserver,
                   nsIDOMEventListener)
 
 nsresult
-nsImageMap::GetBoundsForAreaContent(nsIContent *aContent,
-                                    nsRect& aBounds)
+nsImageMap::GetBoundsForAreaContent(nsIContent *aContent, nsRect& aBounds)
 {
   NS_ENSURE_TRUE(aContent && mImageFrame, NS_ERROR_INVALID_ARG);
 
   // Find the Area struct associated with this content node, and return bounds
-  uint32_t i, n = mAreas.Length();
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
+  for (auto& area : mAreas) {
     if (area->mArea == aContent) {
       aBounds = nsRect();
       area->GetRect(mImageFrame, aBounds);
       return NS_OK;
     }
   }
   return NS_ERROR_FAILURE;
 }
 
 void
+nsImageMap::AreaRemoved(HTMLAreaElement* aArea)
+{
+  if (aArea->IsInUncomposedDoc()) {
+    NS_ASSERTION(aArea->GetPrimaryFrame() == mImageFrame,
+                 "Unexpected primary frame");
+
+    aArea->SetPrimaryFrame(nullptr);
+  }
+
+  aArea->RemoveSystemEventListener(NS_LITERAL_STRING("focus"), this, false);
+  aArea->RemoveSystemEventListener(NS_LITERAL_STRING("blur"), this, false);
+}
+
+void
 nsImageMap::FreeAreas()
 {
-  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;
+  for (UniquePtr<Area>& area : mAreas) {
+    AreaRemoved(area->mArea);
   }
 
   mAreas.Clear();
 }
 
 void
 nsImageMap::Init(nsImageFrame* aImageFrame, nsIContent* aMap)
 {
@@ -793,35 +793,35 @@ nsImageMap::AddArea(HTMLAreaElement* aAr
 {
   static nsIContent::AttrValuesArray strings[] =
     {&nsGkAtoms::rect, &nsGkAtoms::rectangle,
      &nsGkAtoms::circle, &nsGkAtoms::circ,
      &nsGkAtoms::_default,
      &nsGkAtoms::poly, &nsGkAtoms::polygon,
      nullptr};
 
-  Area* area;
+  UniquePtr<Area> area;
   switch (aArea->FindAttrValueIn(kNameSpaceID_None, nsGkAtoms::shape,
                                  strings, eIgnoreCase)) {
   case nsIContent::ATTR_VALUE_NO_MATCH:
   case nsIContent::ATTR_MISSING:
   case 0:
   case 1:
-    area = new RectArea(aArea);
+    area = MakeUnique<RectArea>(aArea);
     break;
   case 2:
   case 3:
-    area = new CircleArea(aArea);
+    area = MakeUnique<CircleArea>(aArea);
     break;
   case 4:
-    area = new DefaultArea(aArea);
+    area = MakeUnique<DefaultArea>(aArea);
     break;
   case 5:
   case 6:
-    area = new PolyArea(aArea);
+    area = MakeUnique<PolyArea>(aArea);
     break;
   default:
     area = nullptr;
     MOZ_ASSERT_UNREACHABLE("FindAttrValueIn returned an unexpected value.");
     break;
   }
 
   //Add focus listener to track area focus changes
@@ -833,24 +833,24 @@ nsImageMap::AddArea(HTMLAreaElement* aAr
   // 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);
+  mAreas.AppendElement(Move(area));
 }
 
 nsIContent*
 nsImageMap::GetArea(nscoord aX, nscoord aY) const
 {
   NS_ASSERTION(mMap, "Not initialized");
-  for (auto* area : mAreas) {
+  for (const auto& area : mAreas) {
     if (area->IsInside(aX, aY)) {
       return area->mArea;
     }
   }
 
   return nullptr;
 }
 
@@ -860,25 +860,23 @@ nsImageMap::GetAreaAt(uint32_t aIndex) c
   return mAreas.ElementAt(aIndex)->mArea;
 }
 
 void
 nsImageMap::Draw(nsIFrame* aFrame, DrawTarget& aDrawTarget,
                  const ColorPattern& aColor,
                  const StrokeOptions& aStrokeOptions)
 {
-  uint32_t i, n = mAreas.Length();
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
+  for (auto& area : mAreas) {
     area->Draw(aFrame, aDrawTarget, aColor, aStrokeOptions);
   }
 }
 
 void
-nsImageMap::MaybeUpdateAreas(nsIContent *aContent)
+nsImageMap::MaybeUpdateAreas(nsIContent* aContent)
 {
   if (aContent == mMap || mConsiderWholeSubtree) {
     UpdateAreas();
   }
 }
 
 void
 nsImageMap::AttributeChanged(nsIDocument*  aDocument,
@@ -920,23 +918,63 @@ nsImageMap::ContentAppended(nsIDocument 
 void
 nsImageMap::ContentInserted(nsIDocument *aDocument,
                             nsIContent* aContainer,
                             nsIContent* aChild)
 {
   MaybeUpdateAreas(aContainer);
 }
 
+static UniquePtr<Area>
+TakeArea(nsImageMap::AreaList& aAreas, HTMLAreaElement* aArea)
+{
+  UniquePtr<Area> result;
+  size_t index = 0;
+  for (UniquePtr<Area>& area : aAreas) {
+    if (area->mArea == aArea) {
+      result = Move(area);
+      break;
+    }
+    index++;
+  }
+
+  if (result) {
+    aAreas.RemoveElementAt(index);
+  }
+
+  return result;
+}
+
 void
 nsImageMap::ContentRemoved(nsIDocument *aDocument,
                            nsIContent* aContainer,
                            nsIContent* aChild,
                            nsIContent* aPreviousSibling)
 {
-  MaybeUpdateAreas(aContainer);
+  if (aContainer != mMap && !mConsiderWholeSubtree) {
+    return;
+  }
+
+  auto* areaElement = HTMLAreaElement::FromContent(aChild);
+  if (!areaElement) {
+    return;
+  }
+
+  UniquePtr<Area> area = TakeArea(mAreas, areaElement);
+  if (!area) {
+    return;
+  }
+
+  AreaRemoved(area->mArea);
+
+#ifdef ACCESSIBILITY
+  if (nsAccessibilityService* accService = GetAccService()) {
+    accService->UpdateImageMap(mImageFrame);
+  }
+#endif
 }
 
 void
 nsImageMap::ParentChainChanged(nsIContent* aContent)
 {
   NS_ASSERTION(aContent == mMap,
                "Unexpected ParentChainChanged notification!");
   if (mImageFrame) {
@@ -954,19 +992,18 @@ nsImageMap::HandleEvent(nsIDOMEvent* aEv
              "Unexpected event type");
 
   //Set which one of our areas changed focus
   nsCOMPtr<nsIContent> targetContent = do_QueryInterface(
     aEvent->InternalDOMEvent()->GetTarget());
   if (!targetContent) {
     return NS_OK;
   }
-  uint32_t i, n = mAreas.Length();
-  for (i = 0; i < n; i++) {
-    Area* area = mAreas.ElementAt(i);
+
+  for (auto& area : mAreas) {
     if (area->mArea == targetContent) {
       //Set or Remove internal focus
       area->HasFocus(focus);
       //Now invalidate the rect
       if (mImageFrame) {
         mImageFrame->InvalidateFrame();
       }
       break;
--- a/layout/generic/nsImageMap.h
+++ b/layout/generic/nsImageMap.h
@@ -77,32 +77,37 @@ public:
   NS_DECL_NSIMUTATIONOBSERVER_PARENTCHAINCHANGED
 
   //nsIDOMEventListener
   NS_DECL_NSIDOMEVENTLISTENER
 
   nsresult GetBoundsForAreaContent(nsIContent *aContent,
                                    nsRect& aBounds);
 
+  using AreaList = AutoTArray<mozilla::UniquePtr<Area>, 8>;
+
 protected:
   virtual ~nsImageMap();
 
   void FreeAreas();
 
   void UpdateAreas();
 
   void SearchForAreas(nsIContent* aParent);
 
   void AddArea(mozilla::dom::HTMLAreaElement* aArea);
+  void AreaRemoved(mozilla::dom::HTMLAreaElement* aArea);
 
   void MaybeUpdateAreas(nsIContent *aContent);
 
   nsImageFrame* mImageFrame;  // the frame that owns us
   nsCOMPtr<nsIContent> mMap;
-  AutoTArray<Area*, 8> mAreas; // almost always has some entries
+
+  // almost always has some entries
+  AreaList mAreas;
 
   // This is set when we search for all area children and tells us whether we
   // should consider the whole subtree or just direct children when we get
   // content notifications about changes inside the map subtree.
   bool mConsiderWholeSubtree;
 };
 
 #endif /* nsImageMap_h */