Use more const references in HTMLCollection / LiveNodeList

Source/core/html/HTMLLabelElement.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2010 Apple Inc. All rights reserved.
  *           (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
@@ skipping 17 matching lines @@
 #include "HTMLNames.h"
 #include "core/dom/ElementTraversal.h"
 #include "core/events/Event.h"
 #include "core/events/ThreadLocalEventNames.h"
 #include "core/html/FormAssociatedElement.h"
 
 namespace WebCore {
 
 using namespace HTMLNames;
 
-static bool supportsLabels(Element* element)
+static bool supportsLabels(const Element& element)
 {
-    if (!element || !element->isHTMLElement())

[Inactive, 2014/01/17 21:10:43]

Got rid of a useless null check.

+    if (!element.isHTMLElement())
         return false;
-    if (!toHTMLElement(element)->isLabelable())
+    if (!toHTMLElement(element).isLabelable())
         return false;
-    return toLabelableElement(element)->supportLabels();
+    return toLabelableElement(element).supportLabels();
 }
 
 inline HTMLLabelElement::HTMLLabelElement(Document& document)
     : HTMLElement(labelTag, document)
 {
     ScriptWrappable::init(this);
 }
 
 PassRefPtr<HTMLLabelElement> HTMLLabelElement::create(Document& document)
 {
     return adoptRef(new HTMLLabelElement(document));
 }
 
 bool HTMLLabelElement::rendererIsFocusable() const
 {
     HTMLLabelElement* that = const_cast<HTMLLabelElement*>(this);
     return that->isContentEditable();
 }
 
-LabelableElement* HTMLLabelElement::control()
+LabelableElement* HTMLLabelElement::control() const
 {
     const AtomicString& controlId = getAttribute(forAttr);
     if (controlId.isNull()) {
         // Search the children and descendants of the label element for a form element.
         // per http://dev.w3.org/html5/spec/Overview.html#the-label-element
         // the form element must be "labelable form-associated element".
-        Element* element = this;
+        Element* element = const_cast<HTMLLabelElement*>(this);

[adamk, 2014/01/21 21:43:55]

I'd prefer restructuring the logic and removing this const_cast (likely by using
a do {} while loop). It'll involve an extra call to ElementTraversal::next(),
but that seems worthwhile to me to avoid const_casting in a const method (which
looks quite suspicious).

[Inactive, 2014/01/21 23:14:04]

Sure, I will refactor to a for loop and thus avoid the const_cast. I agree it
does not cause any additional traversal and the code would be a bit more
readable.

[Inactive, 2014/01/21 23:40:20]

Done.

         while ((element = ElementTraversal::next(*element, this))) {
-            if (!supportsLabels(element))
+            if (!supportsLabels(*element))
                 continue;
             return toLabelableElement(element);
         }
         return 0;
     }
 
     if (Element* element = treeScope().getElementById(controlId)) {
-        if (supportsLabels(element))
+        if (supportsLabels(*element))
             return toLabelableElement(element);
     }
 
     return 0;
 }
 
 HTMLFormElement* HTMLLabelElement::formOwner() const
 {
     return FormAssociatedElement::findAssociatedForm(this, 0);
 }
@@ skipping 92 matching lines @@
 
 void HTMLLabelElement::accessKeyAction(bool sendMouseEvents)
 {
     if (HTMLElement* element = control())
         element->accessKeyAction(sendMouseEvents);
     else
         HTMLElement::accessKeyAction(sendMouseEvents);
 }
 
 } // namespace