Have ElementData::attributeItem() return a reference

Have ElementData::attributeItem() return a reference

Have ElementData::attributeItem() return a reference instead of a pointer as it
can never return null. This makes the calling code look safer.

R=adamk, eseidel

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=168037

[eseidel, 2014-02-26 21:03:28]

lgtm

[commit-bot: I haz the power, 2014-02-26 21:03:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/180723006/1

[commit-bot: I haz the power, 2014-02-27 02:53:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-27 02:53:48]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[Inactive, 2014-02-27 03:26:25]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-02-27 03:26:36]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/180723006/1

[commit-bot: I haz the power, 2014-02-27 03:26:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-02-27 03:26:42]

Failed to apply patch for Source/core/dom/Element.cpp:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/core/dom/Element.cpp
  Hunk #1 succeeded at 309 (offset 1 line).
  Hunk #2 succeeded at 325 (offset 1 line).
  Hunk #3 succeeded at 934 (offset 1 line).
  Hunk #4 FAILED at 963.
  Hunk #5 succeeded at 1251 (offset 1 line).
  Hunk #6 FAILED at 1967.
  Hunk #7 FAILED at 2039.
  Hunk #8 FAILED at 2048.
  Hunk #9 succeeded at 2696 (offset 1 line).
  Hunk #10 succeeded at 3215 (offset 31 lines).
  Hunk #11 succeeded at 3283 (offset 31 lines).
  4 out of 11 hunks FAILED -- saving rejects to file
Source/core/dom/Element.cpp.rej

Patch:       Source/core/dom/Element.cpp
Index: Source/core/dom/Element.cpp
diff --git a/Source/core/dom/Element.cpp b/Source/core/dom/Element.cpp
index
b2e15a9932dd665006270c105e07fef4d5d01e81..5dabd4ff31a4538d9e15a188e1c1cb280213f492
100644
--- a/Source/core/dom/Element.cpp
+++ b/Source/core/dom/Element.cpp
@@ -308,12 +308,12 @@ PassRefPtr<Element>
Element::cloneElementWithoutAttributesAndChildren()
 PassRefPtr<Attr> Element::detachAttribute(size_t index)
 {
     ASSERT(elementData());
-    const Attribute* attribute = elementData()->attributeItem(index);
-    RefPtr<Attr> attrNode = attrIfExists(attribute->name());
+    const Attribute& attribute = elementData()->attributeItem(index);
+    RefPtr<Attr> attrNode = attrIfExists(attribute.name());
     if (attrNode)
         detachAttrNodeAtIndex(attrNode.get(), index);
     else {
-        attrNode = Attr::create(document(), attribute->name(),
attribute->value());
+        attrNode = Attr::create(document(), attribute.name(),
attribute.value());
         removeAttributeInternal(index, NotInSynchronizationOfLazyAttribute);
     }
     return attrNode.release();
@@ -324,10 +324,9 @@ void Element::detachAttrNodeAtIndex(Attr* attr, size_t
index)
     ASSERT(attr);
     ASSERT(elementData());
 
-    const Attribute* attribute = elementData()->attributeItem(index);
-    ASSERT(attribute);
-    ASSERT(attribute->name() == attr->qualifiedName());
-    detachAttrNodeFromElementWithValue(attr, attribute->value());
+    const Attribute& attribute = elementData()->attributeItem(index);
+    ASSERT(attribute.name() == attr->qualifiedName());
+    detachAttrNodeFromElementWithValue(attr, attribute.value());
     removeAttributeInternal(index, NotInSynchronizationOfLazyAttribute);
 }
 
@@ -934,7 +933,7 @@ void Element::setAttribute(const AtomicString& localName,
const AtomicString& va
     const AtomicString& caseAdjustedLocalName = shouldIgnoreAttributeCase() ?
localName.lower() : localName;
 
     size_t index = elementData() ?
elementData()->getAttributeItemIndex(caseAdjustedLocalName, false) : kNotFound;
-    const QualifiedName& qName = index != kNotFound ?
attributeItem(index)->name() : QualifiedName(nullAtom, caseAdjustedLocalName,
nullAtom);
+    const QualifiedName& qName = index != kNotFound ?
attributeItem(index).name() : QualifiedName(nullAtom, caseAdjustedLocalName,
nullAtom);
     setAttributeInternal(index, qName, value,
NotInSynchronizationOfLazyAttribute);
 }
 
@@ -964,20 +963,20 @@ ALWAYS_INLINE void Element::setAttributeInternal(size_t
index, const QualifiedNa
         return;
     }
 
-    const Attribute* existingAttribute = attributeItem(index);
-    QualifiedName existingAttributeName = existingAttribute->name();
+    const Attribute& existingAttribute = attributeItem(index);
+    QualifiedName existingAttributeName = existingAttribute.name();
 
     if (!inSynchronizationOfLazyAttribute)
-        willModifyAttribute(existingAttributeName, existingAttribute->value(),
newValue);
+        willModifyAttribute(existingAttributeName, existingAttribute.value(),
newValue);
 
-    if (newValue != existingAttribute->value()) {
+    if (newValue != existingAttribute.value()) {
         // If there is an Attr node hooked to this attribute, the
Attr::setValue() call below
         // will write into the ElementData.
         // FIXME: Refactor this so it makes some sense.
         if (RefPtr<Attr> attrNode = inSynchronizationOfLazyAttribute ? nullptr
: attrIfExists(existingAttributeName))
             attrNode->setValue(newValue);
         else
-           
ensureUniqueElementData()->attributeItem(index)->setValue(newValue);
+            ensureUniqueElementData()->attributeItem(index).setValue(newValue);
     }
 
     if (!inSynchronizationOfLazyAttribute)
@@ -1251,10 +1250,10 @@ const AtomicString& Element::locateNamespacePrefix(const
AtomicString& namespace
     if (hasAttributes()) {
         unsigned attributeCount = this->attributeCount();
         for (unsigned i = 0; i < attributeCount; ++i) {
-            const Attribute* attr = attributeItem(i);
+            const Attribute& attr = attributeItem(i);
 
-            if (attr->prefix() == xmlnsAtom && attr->value() ==
namespaceToLocate)
-                return attr->localName();
+            if (attr.prefix() == xmlnsAtom && attr.value() ==
namespaceToLocate)
+                return attr.localName();
         }
     }
 
@@ -1968,9 +1967,9 @@ PassRefPtr<Attr> Element::setAttributeNode(Attr* attrNode,
ExceptionState& excep
     size_t index =
elementData->getAttributeItemIndex(attrNode->qualifiedName(),
shouldIgnoreAttributeCase());
     if (index != kNotFound) {
         if (oldAttrNode)
-            detachAttrNodeFromElementWithValue(oldAttrNode.get(),
elementData->attributeItem(index)->value());
+            detachAttrNodeFromElementWithValue(oldAttrNode.get(),
elementData->attributeItem(index).value());
         else
-            oldAttrNode = Attr::create(document(), attrNode->qualifiedName(),
elementData->attributeItem(index)->value());
+            oldAttrNode = Attr::create(document(), attrNode->qualifiedName(),
elementData->attributeItem(index).value());
     }
 
     setAttributeInternal(index, attrNode->qualifiedName(), attrNode->value(),
NotInSynchronizationOfLazyAttribute);
@@ -2040,8 +2039,8 @@ void Element::removeAttributeInternal(size_t index,
SynchronizationOfLazyAttribu
 
     UniqueElementData* elementData = ensureUniqueElementData();
 
-    QualifiedName name = elementData->attributeItem(index)->name();
-    AtomicString valueBeingRemoved =
elementData->attributeItem(index)->value();
+    QualifiedName name = elementData->attributeItem(index).name();
+    AtomicString valueBeingRemoved = elementData->attributeItem(index).value();
 
     if (!inSynchronizationOfLazyAttribute) {
         if (!valueBeingRemoved.isNull())
@@ -2049,7 +2048,7 @@ void Element::removeAttributeInternal(size_t index,
SynchronizationOfLazyAttribu
     }
 
     if (RefPtr<Attr> attrNode = attrIfExists(name))
-        detachAttrNodeFromElementWithValue(attrNode.get(),
elementData->attributeItem(index)->value());
+        detachAttrNodeFromElementWithValue(attrNode.get(),
elementData->attributeItem(index).value());
 
     elementData->removeAttribute(index);
 
@@ -2696,7 +2695,7 @@ void Element::normalizeAttributes()
     // attributeCount() cannot be cached before the loop because the attributes
     // list is altered while iterating.
     for (unsigned i = 0; i < attributeCount(); ++i) {
-        if (RefPtr<Attr> attr = attrIfExists(attributeItem(i)->name()))
+        if (RefPtr<Attr> attr = attrIfExists(attributeItem(i).name()))
             attr->normalize();
     }
 }
@@ -3185,9 +3184,9 @@ void Element::detachAllAttrNodesFromElement()
 
     unsigned attributeCount = this->attributeCount();
     for (unsigned i = 0; i < attributeCount; ++i) {
-        const Attribute* attribute = attributeItem(i);
-        if (RefPtr<Attr> attrNode = findAttrNodeInList(*attrNodeList,
attribute->name()))
-            attrNode->detachFromElementWithValue(attribute->value());
+        const Attribute& attribute = attributeItem(i);
+        if (RefPtr<Attr> attrNode = findAttrNodeInList(*attrNodeList,
attribute.name()))
+            attrNode->detachFromElementWithValue(attribute.value());
     }
 
     removeAttrNodeListForElement(this);
@@ -3253,8 +3252,8 @@ void Element::cloneAttributesFromElement(const Element&
other)
 
     unsigned length = m_elementData->length();
     for (unsigned i = 0; i < length; ++i) {
-        const Attribute* attribute = const_cast<const
ElementData*>(m_elementData.get())->attributeItem(i);
-        attributeChangedFromParserOrByCloning(attribute->name(),
attribute->value(), ModifiedByCloning);
+        const Attribute& attribute = m_elementData->attributeItem(i);
+        attributeChangedFromParserOrByCloning(attribute.name(),
attribute.value(), ModifiedByCloning);
     }
 }

[Inactive, 2014-02-27 14:15:13]

The CQ bit was checked by ch.dumez@samsung.com

[commit-bot: I haz the power, 2014-02-27 14:15:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ch.dumez@samsung.com/180723006/20001

[commit-bot: I haz the power, 2014-02-27 16:19:52]

Change committed as 168037