[webkit-reviews] review requested: [Bug 62302] Remove "const" from obviously non-const accessors : [Attachment 97654] First patch v1

[bugzilla-daemon at webkit.org]

Peter Kasting <pkasting at google.com> has asked  for review:
Bug 62302: Remove "const" from obviously non-const accessors
https://bugs.webkit.org/show_bug.cgi?id=62302

Attachment 97654: First patch v1
https://bugs.webkit.org/attachment.cgi?id=97654&action=review

------- Additional Comments from Peter Kasting <pkasting at google.com>
I also covered this in the ChangeLog, but for ease of reading/sorting, below
are the functions made non-const, grouped as to why.  Sorry there's so many of
these, but like most const-related patches, you wind up fixing chains of
functions from a few source points :).	Please do look through these to see if
I have any bogus rationale.

One unresolved question: Is it possible to provide a const version of
Document::images() (and similar nearby accessors) that returns a collection of
const pointers/objects that cannot be used to mutate the tree?	If so, we can
avoid making some imageElement() accessors necessarily-non-const in this patch.



Functions marked newly non-const because they call existing functions with side
effects:

Element::boundsInWindowSpace(): Calls
Document::updateLayoutIgnorePendingStylesheets()
Element::cloneElementWithoutAttributesAndChildren(): Calls
Document::createElement()
Element::getBoundingClientRect(): Calls
Document::updateLayoutIgnorePendingStylesheets()
Element::getClientRects(): Calls
Document::updateLayoutIgnorePendingStylesheets()
Element::innerText(): Calls Document::updateLayoutIgnorePendingStylesheets()
Element::scrollHeight(): Calls Document::updateLayoutIgnorePendingStylesheets()

Element::scrollLeft(): Calls Document::updateLayoutIgnorePendingStylesheets()
Element::scrollTop(): Calls Document::updateLayoutIgnorePendingStylesheets()
Element::scrollWidth(): Calls Document::updateLayoutIgnorePendingStylesheets()
HTMLAppletElement::renderWidgetForJSBindings(): Calls
RenderApplet::createWidgetIfNecessary()
HTMLBodyElement::scrollHeight(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLBodyElement::scrollLeft(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLBodyElement::scrollTop(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLBodyElement::scrollWidth(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLEmbedElement::renderWidgetForJSBindings(): Calls Document::createElement()
HTMLFrameElementBase::height(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLFrameElementBase::width(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLImageElement::height():  Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLImageElement::width(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLMapElement::imageElement(): Calls Document::images()
HTMLObjectElement::renderWidgetForJSBindings(): Calls
Document::updateLayoutIgnorePendingStylesheets()
HTMLOptionElement::selected(): Calls
HTMLSelectElement::recalcListItemsIfNeeded()
Node::isContentEditable(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGElement::accessDocumentSVGExtensions():: Calls
Document::accessSVGExtensions()
SVGSMILElement::targetElement(): Calls Document::accessSVGExtensions()
SVGStyledElement::buildPendingResourcesIfNeeded(): Calls
Document::accessSVGExtensions()
SVGTextContentElement::getCharNumAtPosition(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getComputedTextLength(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getEndPositionOfChar(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getExtentOfChar(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getNumberOfChars(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getRotationOfChar(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getStartPositionOfChar(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGTextContentElement::getSubStringLength(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGUseElement::instanceRoot(): Calls
Document::updateLayoutIgnorePendingStylesheets()
SVGUseElement::toClipPath(): Calls Document::accessSVGExtensions()


Static functions with a newly-non-const argument, for similar reasons:

static SVGLocatable::computeCTM(): Calls
Document::updateLayoutIgnorePendingStylesheets() on arg
static SVGLocatable::getBBox(): Calls
Document::updateLayoutIgnorePendingStylesheets() on arg


Virtual functions marked non-const because other implementations in the
inheritance hierarchy are newly non-const:

HTMLInputElement::shouldUseInputMethod(): Node version is not const
HTMLPlugInElement::renderWidgetForJSBindings: Various subclass versions are not
const
HTMLScriptElement::cloneElementWithoutAttributesAndChildren(): Element version
is not const
HTMLTextAreaElement::shouldUseInputMethod(): Node version is not const
KeygenSelectElement::cloneElementWithoutAttributesAndChildren(): Element
version is not const
OptionElement::selected(): HTMLOptionElement version is not const
SliderThumbElement::cloneElementWithoutAttributesAndChildren(): Element version
is not const
SVGLocatable::getBBox(): Various subclass versions are not const
SVGLocatable::getCTM(): Various subclass versions are not const
SVGLocatable::getScreenCTM(): Various subclass versions are not const
SVGScriptElement::cloneElementWithoutAttributesAndChildren(): Element version
is not const
SVGShadowTreeContainerElement::cloneElementWithoutAttributesOrChildren():
Element version is not const
SVGSMILElement::hasValidAttributeType(): Various subclass versions are not
const
SVGStyledTransformableElement::toClipPath(): SVGUseElement version is not const



Functions marked newly non-const because they call other functions newly marked
non-const:

Element::outerText(): Calls Element::innerText()
HTMLAnchorElement::text(): Calls Element::innerText()
HTMLAreaElement::imageElement(): Calls HTMLMapElement::imageElement()
HTMLPlugInElement::getInstance(): Calls HTMLPlugInElement::pluginWidget()
HTMLPlugInElement::pluginWidget(): Calls
HTMLPlugInElement::renderWidgetForJSBindings()
Node::shouldUseInputMethod(): Calls Node::isContentEditable()
SVGAnimateElement::hasValidAttributeType(): Calls
SVGSMILElement::targetElement()
SVGAnimateMotionElement::hasValidAttributeType(): Calls
SVGSMILElement::targetElement()
SVGAnimateTransformElement::hasValidAttributeType(): Calls
SVGSMILElement::targetElement()
SVGElement::boundingBox(): Calls SVGStyledLocatableElement::getBBox()
SVGLocatable::getTransformToElement(): Calls SVGLocatable::getCTM()
SVGSMILElement::eventBaseFor(): Calls SVGSMILElement::targetElement()
SVGStyledLocatableElement::getBBox(): Calls static SVGLocatable::getBBox()
SVGStyledLocatableElement::getCTM(): Calls static SVGLocatable::computeCTM()
SVGStyledLocatableElement::getScreenCTM(): Calls static
SVGLocatable::computeCTM()
SVGStyledTransformableElement::getCTM(): Calls static
SVGLocatable::computeCTM()
SVGStyledTransformableElement::getScreenCTM(): Calls static
SVGLocatable::computeCTM()
SVGStyledTransformableElement::getBBox(): Calls static SVGLocatable::getBBox()
SVGTextContentElement::selectSubString(): Calls
SVGTextContentElement::getNumberOfChars()
SVGTextElement::getBBox(): Calls static SVGLocatable::getBBox()
SVGTextElement::getCTM(): Calls static SVGLocatable::computeCTM()
SVGTextElement::getScreenCTM(): Calls static SVGLocatable::computeCTM()
WebElement::innerText(): Calls Element::innerText()


Finally, note the above reasons are not necessarily complete explanations, e.g.
the cloneElementWithoutAttributesAndChildren() functions also generally create
new objects, return pointers that allow tree mutation, etc.