Skip to content

Commit

Permalink
Bug 909254 - Stop using jsapi for HTMLCollection.namedItem; r=peterv
Browse files Browse the repository at this point in the history
  • Loading branch information
Ms2ger committed Nov 11, 2013
1 parent ecba101 commit 97b7fef
Show file tree
Hide file tree
Showing 12 changed files with 73 additions and 110 deletions.
24 changes: 3 additions & 21 deletions content/base/src/nsContentList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "nsGkAtoms.h"
#include "mozilla/dom/HTMLCollectionBinding.h"
#include "mozilla/dom/NodeListBinding.h"
#include "mozilla/dom/BindingUtils.h"
#include "mozilla/Likely.h"
#include "nsGenericHTMLElement.h"
#include <algorithm>
Expand All @@ -38,6 +37,7 @@
#define ASSERT_IN_SYNC PR_BEGIN_MACRO PR_END_MACRO
#endif

using namespace mozilla;
using namespace mozilla::dom;

nsBaseContentList::~nsBaseContentList()
Expand Down Expand Up @@ -535,7 +535,7 @@ nsContentList::Item(uint32_t aIndex, bool aDoFlush)
return mElements.SafeElementAt(aIndex);
}

nsIContent *
Element*
nsContentList::NamedItem(const nsAString& aName, bool aDoFlush)
{
BringSelfUpToDate(aDoFlush);
Expand All @@ -554,7 +554,7 @@ nsContentList::NamedItem(const nsAString& aName, bool aDoFlush)
name, eCaseMatters) ||
content->AttrValueIs(kNameSpaceID_None, nsGkAtoms::id,
name, eCaseMatters))) {
return content;
return content->AsElement();
}
}

Expand Down Expand Up @@ -672,24 +672,6 @@ nsContentList::Item(uint32_t aIndex)
return GetElementAt(aIndex);
}

JSObject*
nsContentList::NamedItem(JSContext* cx, const nsAString& name,
mozilla::ErrorResult& error)
{
nsIContent *item = NamedItem(name, true);
if (!item) {
return nullptr;
}
JS::Rooted<JSObject*> wrapper(cx, GetWrapper());
JSAutoCompartment ac(cx, wrapper);
JS::Rooted<JS::Value> v(cx);
if (!mozilla::dom::WrapObject(cx, wrapper, item, item, nullptr, &v)) {
error.Throw(NS_ERROR_FAILURE);
return nullptr;
}
return &v.toObject();
}

void
nsContentList::AttributeChanged(nsIDocument *aDocument, Element* aElement,
int32_t aNameSpaceID, nsIAtom* aAttribute,
Expand Down
12 changes: 9 additions & 3 deletions content/base/src/nsContentList.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,14 +269,20 @@ class nsContentList : public nsBaseContentList,

virtual nsIContent* Item(uint32_t aIndex) MOZ_OVERRIDE;
virtual mozilla::dom::Element* GetElementAt(uint32_t index) MOZ_OVERRIDE;
virtual JSObject* NamedItem(JSContext* cx, const nsAString& name,
mozilla::ErrorResult& error) MOZ_OVERRIDE;
virtual mozilla::dom::Element*
GetFirstNamedElement(const nsAString& aName, bool& aFound) MOZ_OVERRIDE
{
mozilla::dom::Element* item = NamedItem(aName, true);
aFound = !!item;
return item;
}
virtual void GetSupportedNames(nsTArray<nsString>& aNames) MOZ_OVERRIDE;

// nsContentList public methods
NS_HIDDEN_(uint32_t) Length(bool aDoFlush);
NS_HIDDEN_(nsIContent*) Item(uint32_t aIndex, bool aDoFlush);
NS_HIDDEN_(nsIContent*) NamedItem(const nsAString& aName, bool aDoFlush);
NS_HIDDEN_(mozilla::dom::Element*)
NamedItem(const nsAString& aName, bool aDoFlush);

// nsIMutationObserver
NS_DECL_NSIMUTATIONOBSERVER_ATTRIBUTECHANGED
Expand Down
16 changes: 9 additions & 7 deletions content/html/content/public/nsIHTMLCollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,15 +60,17 @@ class nsIHTMLCollection : public nsIDOMHTMLCollection
aFound = !!item;
return item;
}
virtual JSObject* NamedItem(JSContext* cx, const nsAString& name,
mozilla::ErrorResult& error) = 0;
JSObject* NamedGetter(JSContext* cx, const nsAString& name,
bool& found, mozilla::ErrorResult& error)
mozilla::dom::Element* NamedItem(const nsAString& aName)
{
JSObject* namedItem = NamedItem(cx, name, error);
found = !!namedItem;
return namedItem;
bool dummy;
return NamedGetter(aName, dummy);
}
mozilla::dom::Element* NamedGetter(const nsAString& aName, bool& aFound)
{
return GetFirstNamedElement(aName, aFound);
}
virtual mozilla::dom::Element*
GetFirstNamedElement(const nsAString& aName, bool& aFound) = 0;

virtual void GetSupportedNames(nsTArray<nsString>& aNames) = 0;

Expand Down
26 changes: 14 additions & 12 deletions content/html/content/src/HTMLFormControlsCollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,22 +343,24 @@ HTMLFormControlsCollection::GetParentObject()
return mForm;
}

JSObject*
HTMLFormControlsCollection::NamedItem(JSContext* aCx, const nsAString& aName,
ErrorResult& aError)
/* virtual */ Element*
HTMLFormControlsCollection::GetFirstNamedElement(const nsAString& aName, bool& aFound)
{
nsISupports* item = NamedItemInternal(aName, true);
if (!item) {
Nullable<OwningNodeListOrElement> maybeResult;
NamedGetter(aName, aFound, maybeResult);
if (!aFound) {
return nullptr;
}
JS::Rooted<JSObject*> wrapper(aCx, nsWrapperCache::GetWrapper());
JSAutoCompartment ac(aCx, wrapper);
JS::Rooted<JS::Value> v(aCx);
if (!dom::WrapObject(aCx, wrapper, item, &v)) {
aError.Throw(NS_ERROR_FAILURE);
return nullptr;
MOZ_ASSERT(!maybeResult.IsNull());
const OwningNodeListOrElement& result = maybeResult.Value();
if (result.IsElement()) {
return result.GetAsElement().get();
}
if (result.IsNodeList()) {
nsINodeList& nodelist = result.GetAsNodeList();
return nodelist.Item(0)->AsElement();
}
return &v.toObject();
MOZ_ASSUME_UNREACHABLE("Should only have Elements and NodeLists here.");
}

void
Expand Down
7 changes: 4 additions & 3 deletions content/html/content/src/HTMLFormControlsCollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ class HTMLFormControlsCollection : public nsIHTMLCollection
virtual Element* GetElementAt(uint32_t index);
virtual nsINode* GetParentObject() MOZ_OVERRIDE;

virtual JSObject* NamedItem(JSContext* aCx, const nsAString& aName,
ErrorResult& aError);
virtual void GetSupportedNames(nsTArray<nsString>& aNames);
virtual Element*
GetFirstNamedElement(const nsAString& aName, bool& aFound) MOZ_OVERRIDE;

void
NamedGetter(const nsAString& aName,
bool& aFound,
Expand All @@ -54,6 +54,7 @@ class HTMLFormControlsCollection : public nsIHTMLCollection
bool dummy;
NamedGetter(aName, dummy, aResult);
}
virtual void GetSupportedNames(nsTArray<nsString>& aNames);

nsresult AddElementToTable(nsGenericHTMLFormElement* aChild,
const nsAString& aName);
Expand Down
22 changes: 3 additions & 19 deletions content/html/content/src/HTMLOptionsCollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ HTMLOptionsCollection::GetElementAt(uint32_t aIndex)
}

HTMLOptionElement*
HTMLOptionsCollection::GetNamedItem(const nsAString& aName) const
HTMLOptionsCollection::NamedGetter(const nsAString& aName, bool& aFound)
{
uint32_t count = mElements.Length();
for (uint32_t i = 0; i < count; i++) {
Expand All @@ -258,10 +258,12 @@ HTMLOptionsCollection::GetNamedItem(const nsAString& aName) const
eCaseMatters) ||
content->AttrValueIs(kNameSpaceID_None, nsGkAtoms::id, aName,
eCaseMatters))) {
aFound = true;
return content;
}
}

aFound = false;
return nullptr;
}

Expand All @@ -280,24 +282,6 @@ HTMLOptionsCollection::NamedItem(const nsAString& aName,
return NS_OK;
}

JSObject*
HTMLOptionsCollection::NamedItem(JSContext* cx, const nsAString& name,
ErrorResult& error)
{
nsINode* item = GetNamedItem(name);
if (!item) {
return nullptr;
}
JS::Rooted<JSObject*> wrapper(cx, nsWrapperCache::GetWrapper());
JSAutoCompartment ac(cx, wrapper);
JS::Rooted<JS::Value> v(cx);
if (!mozilla::dom::WrapObject(cx, wrapper, item, item, nullptr, &v)) {
error.Throw(NS_ERROR_FAILURE);
return nullptr;
}
return &v.toObject();
}

void
HTMLOptionsCollection::GetSupportedNames(nsTArray<nsString>& aNames)
{
Expand Down
14 changes: 11 additions & 3 deletions content/html/content/src/HTMLOptionsCollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,17 @@ class HTMLOptionsCollection : public nsIHTMLCollection
int32_t aStartIndex, bool aForward,
int32_t* aIndex);

HTMLOptionElement* GetNamedItem(const nsAString& aName) const;
virtual JSObject* NamedItem(JSContext* aCx, const nsAString& aName,
ErrorResult& error) MOZ_OVERRIDE;
HTMLOptionElement* GetNamedItem(const nsAString& aName)
{
bool dummy;
return NamedGetter(aName, dummy);
}
HTMLOptionElement* NamedGetter(const nsAString& aName, bool& aFound);
virtual Element*
GetFirstNamedElement(const nsAString& aName, bool& aFound) MOZ_OVERRIDE
{
return NamedGetter(aName, aFound);
}

void Add(const HTMLOptionOrOptGroupElement& aElement,
const Nullable<HTMLElementOrLong>& aBefore,
Expand Down
10 changes: 0 additions & 10 deletions content/html/content/src/HTMLPropertiesCollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,16 +127,6 @@ HTMLPropertiesCollection::NamedItem(const nsAString& aName,
return NS_OK;
}

JSObject*
HTMLPropertiesCollection::NamedItem(JSContext* cx, const nsAString& name,
mozilla::ErrorResult& error)
{
// HTMLPropertiesCollection.namedItem and the named getter call the NamedItem
// that returns a PropertyNodeList, calling HTMLCollection.namedItem doesn't
// make sense so this returns null.
return nullptr;
}

Element*
HTMLPropertiesCollection::GetElementAt(uint32_t aIndex)
{
Expand Down
12 changes: 10 additions & 2 deletions content/html/content/src/HTMLPropertiesCollection.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,16 @@ class HTMLPropertiesCollection : public nsIHTMLCollection,

void SetDocument(nsIDocument* aDocument);
nsINode* GetParentObject() MOZ_OVERRIDE;
virtual JSObject* NamedItem(JSContext* cx, const nsAString& name,
mozilla::ErrorResult& error) MOZ_OVERRIDE;

virtual Element*
GetFirstNamedElement(const nsAString& aName, bool& aFound) MOZ_OVERRIDE
{
// HTMLPropertiesCollection.namedItem and the named getter call the
// NamedItem that returns a PropertyNodeList, calling
// HTMLCollection.namedItem doesn't make sense so this returns null.
aFound = false;
return nullptr;
}
PropertyNodeList* NamedItem(const nsAString& aName);
PropertyNodeList* NamedGetter(const nsAString& aName, bool& aFound)
{
Expand Down
35 changes: 9 additions & 26 deletions content/html/content/src/HTMLTableElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include "nsRuleData.h"
#include "nsHTMLStyleSheet.h"
#include "nsMappedAttributes.h"
#include "mozilla/dom/BindingUtils.h"
#include "mozilla/dom/UnionTypes.h"
#include "mozilla/dom/HTMLCollectionBinding.h"
#include "mozilla/dom/HTMLTableElementBinding.h"
#include "nsContentUtils.h"
Expand Down Expand Up @@ -41,8 +41,8 @@ class TableRowsCollection : public nsIHTMLCollection,
return mParent;
}

virtual JSObject* NamedItem(JSContext* cx, const nsAString& name,
ErrorResult& error);
virtual Element*
GetFirstNamedElement(const nsAString& aName, bool& aFound) MOZ_OVERRIDE;
virtual void GetSupportedNames(nsTArray<nsString>& aNames);

NS_IMETHOD ParentDestroyed();
Expand Down Expand Up @@ -215,31 +215,14 @@ TableRowsCollection::Item(uint32_t aIndex, nsIDOMNode** aReturn)
return CallQueryInterface(node, aReturn);
}

JSObject*
TableRowsCollection::NamedItem(JSContext* cx, const nsAString& name,
ErrorResult& error)
Element*
TableRowsCollection::GetFirstNamedElement(const nsAString& aName, bool& aFound)
{
aFound = false;
DO_FOR_EACH_ROWGROUP(
nsCOMPtr<nsIHTMLCollection> collection = do_QueryInterface(rows);
if (collection) {
// We'd like to call the nsIHTMLCollection::NamedItem that returns a
// JSObject*, but that relies on collection having a cached wrapper, which
// we can't guarantee here.
nsCOMPtr<nsIDOMNode> item;
error = collection->NamedItem(name, getter_AddRefs(item));
if (error.Failed()) {
return nullptr;
}
if (item) {
JS::Rooted<JSObject*> wrapper(cx, nsWrapperCache::GetWrapper());
JSAutoCompartment ac(cx, wrapper);
JS::Rooted<JS::Value> v(cx);
if (!mozilla::dom::WrapObject(cx, wrapper, item, &v)) {
error.Throw(NS_ERROR_FAILURE);
return nullptr;
}
return &v.toObject();
}
Element* item = rows->NamedGetter(aName, aFound);
if (aFound) {
return item;
}
);
return nullptr;
Expand Down
3 changes: 1 addition & 2 deletions dom/webidl/HTMLCollection.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,5 @@
interface HTMLCollection {
readonly attribute unsigned long length;
getter Element? item(unsigned long index);
[Throws]
getter object? namedItem(DOMString name); // only returns Element
getter Element? namedItem(DOMString name);
};
2 changes: 0 additions & 2 deletions dom/webidl/HTMLOptionsCollection.webidl
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
interface HTMLOptionsCollection : HTMLCollection {
attribute unsigned long length;
[Throws]
getter object? namedItem(DOMString name);
[Throws]
setter creator void (unsigned long index, HTMLOptionElement? option);
[Throws]
void add((HTMLOptionElement or HTMLOptGroupElement) element, optional (HTMLElement or long)? before = null);
Expand Down

0 comments on commit 97b7fef

Please sign in to comment.