Skip to content

Commit

Permalink
Avoid WTF::Vector::at() and operator[] in core/html.
Browse files Browse the repository at this point in the history
at() and operator[] are slow due to RELEASE_ASSERT. We can avoid the slowness by
range-based |for|.

This CL has no behavior changes except runtime performance.

BUG=668300

Review-Url: https://codereview.chromium.org/2556043002
Cr-Commit-Position: refs/heads/master@{#437175}
  • Loading branch information
tkent-google authored and Commit bot committed Dec 8, 2016
1 parent f76ef08 commit f5775c2
Show file tree
Hide file tree
Showing 29 changed files with 158 additions and 192 deletions.
8 changes: 4 additions & 4 deletions third_party/WebKit/Source/core/html/HTMLCollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,13 +512,13 @@ void HTMLCollection::namedItems(const AtomicString& name,

const NamedItemCache& cache = namedItemCache();
if (HeapVector<Member<Element>>* idResults = cache.getElementsById(name)) {
for (unsigned i = 0; i < idResults->size(); ++i)
result.append(idResults->at(i));
for (const auto& element : *idResults)
result.append(element);
}
if (HeapVector<Member<Element>>* nameResults =
cache.getElementsByName(name)) {
for (unsigned i = 0; i < nameResults->size(); ++i)
result.append(nameResults->at(i));
for (const auto& element : *nameResults)
result.append(element);
}
}

Expand Down
5 changes: 2 additions & 3 deletions third_party/WebKit/Source/core/html/HTMLElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -419,9 +419,8 @@ const AtomicString& HTMLElement::eventNameForAttributeName(
{onwheelAttr, EventTypeNames::wheel},
};

for (size_t i = 0; i < WTF_ARRAY_LENGTH(attrToEventNames); i++)
attributeNameToEventNameMap.set(attrToEventNames[i].attr.localName(),
attrToEventNames[i].event);
for (const auto& name : attrToEventNames)
attributeNameToEventNameMap.set(name.attr.localName(), name.event);
}

return attributeNameToEventNameMap.get(attrName.localName());
Expand Down
16 changes: 5 additions & 11 deletions third_party/WebKit/Source/core/html/HTMLFormControlsCollection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,9 @@ static HTMLElement* firstNamedItem(const ListedElement::List& elementsArray,
const String& name) {
DCHECK(attrName == idAttr || attrName == nameAttr);

for (unsigned i = 0; i < elementsArray.size(); ++i) {
HTMLElement* element = toHTMLElement(elementsArray[i]);
if (elementsArray[i]->isEnumeratable() &&
for (const auto& listedElement : elementsArray) {
HTMLElement* element = toHTMLElement(listedElement);
if (listedElement->isEnumeratable() &&
element->fastGetAttribute(attrName) == name)
return element;
}
Expand All @@ -136,10 +136,7 @@ void HTMLFormControlsCollection::updateIdNameCache() const {
NamedItemCache* cache = NamedItemCache::create();
HashSet<StringImpl*> foundInputElements;

const ListedElement::List& elementsArray = listedElements();

for (unsigned i = 0; i < elementsArray.size(); ++i) {
ListedElement* listedElement = elementsArray[i];
for (const auto& listedElement : listedElements()) {
if (listedElement->isEnumeratable()) {
HTMLElement* element = toHTMLElement(listedElement);
const AtomicString& idAttrVal = element->getIdAttribute();
Expand All @@ -158,10 +155,7 @@ void HTMLFormControlsCollection::updateIdNameCache() const {
// HTMLFormControlsCollection doesn't support named getter for IMG
// elements. However we still need to handle IMG elements here because
// HTMLFormElement named getter relies on this.
const HeapVector<Member<HTMLImageElement>>& imageElementsArray =
formImageElements();
for (unsigned i = 0; i < imageElementsArray.size(); ++i) {
HTMLImageElement* element = imageElementsArray[i];
for (const auto& element : formImageElements()) {
const AtomicString& idAttrVal = element->getIdAttribute();
const AtomicString& nameAttrVal = element->getNameAttribute();
if (!idAttrVal.isEmpty() && !foundInputElements.contains(idAttrVal.impl()))
Expand Down
44 changes: 18 additions & 26 deletions third_party/WebKit/Source/core/html/HTMLFormElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,10 +194,9 @@ void HTMLFormElement::handleLocalEvents(Event& event) {
}

unsigned HTMLFormElement::length() const {
const ListedElement::List& elements = listedElements();
unsigned len = 0;
for (unsigned i = 0; i < elements.size(); ++i) {
if (elements[i]->isEnumeratable())
for (const auto& element : listedElements()) {
if (element->isEnumeratable())
++len;
}
return len;
Expand All @@ -211,12 +210,10 @@ void HTMLFormElement::submitImplicitly(Event* event,
bool fromImplicitSubmissionTrigger) {
int submissionTriggerCount = 0;
bool seenDefaultButton = false;
const ListedElement::List& elements = listedElements();
for (unsigned i = 0; i < elements.size(); ++i) {
ListedElement* ListedElement = elements[i];
if (!ListedElement->isFormControlElement())
for (const auto& element : listedElements()) {
if (!element->isFormControlElement())
continue;
HTMLFormControlElement* control = toHTMLFormControlElement(ListedElement);
HTMLFormControlElement* control = toHTMLFormControlElement(element);
if (!seenDefaultButton && control->canBeSuccessfulSubmitButton()) {
if (fromImplicitSubmissionTrigger)
seenDefaultButton = true;
Expand All @@ -238,10 +235,9 @@ void HTMLFormElement::submitImplicitly(Event* event,

bool HTMLFormElement::validateInteractively() {
UseCounter::count(document(), UseCounter::FormValidationStarted);
const ListedElement::List& elements = listedElements();
for (unsigned i = 0; i < elements.size(); ++i) {
if (elements[i]->isFormControlElement())
toHTMLFormControlElement(elements[i])->hideVisibleValidationMessage();
for (const auto& element : listedElements()) {
if (element->isFormControlElement())
toHTMLFormControlElement(element)->hideVisibleValidationMessage();
}

HeapVector<Member<HTMLFormControlElement>> unhandledInvalidControls;
Expand All @@ -257,8 +253,7 @@ bool HTMLFormElement::validateInteractively() {
document().updateStyleAndLayoutIgnorePendingStylesheets();

// Focus on the first focusable control and show a validation message.
for (unsigned i = 0; i < unhandledInvalidControls.size(); ++i) {
HTMLFormControlElement* unhandled = unhandledInvalidControls[i].get();
for (const auto& unhandled : unhandledInvalidControls) {
if (unhandled->isFocusable()) {
unhandled->showValidationMessage();
UseCounter::count(document(), UseCounter::FormValidationShowedMessage);
Expand All @@ -267,8 +262,7 @@ bool HTMLFormElement::validateInteractively() {
}
// Warn about all of unfocusable controls.
if (document().frame()) {
for (unsigned i = 0; i < unhandledInvalidControls.size(); ++i) {
HTMLFormControlElement* unhandled = unhandledInvalidControls[i].get();
for (const auto& unhandled : unhandledInvalidControls) {
if (unhandled->isFocusable())
continue;
String message(
Expand Down Expand Up @@ -471,10 +465,9 @@ void HTMLFormElement::reset() {
return;
}

const ListedElement::List& elements = listedElements();
for (unsigned i = 0; i < elements.size(); ++i) {
if (elements[i]->isFormControlElement())
toHTMLFormControlElement(elements[i])->reset();
for (const auto& element : listedElements()) {
if (element->isFormControlElement())
toHTMLFormControlElement(element)->reset();
}

m_isInResetFunction = false;
Expand Down Expand Up @@ -664,13 +657,12 @@ bool HTMLFormElement::checkInvalidControlsAndCollectUnhandled(
const ListedElement::List& listedElements = this->listedElements();
HeapVector<Member<ListedElement>> elements;
elements.reserveCapacity(listedElements.size());
for (unsigned i = 0; i < listedElements.size(); ++i)
elements.append(listedElements[i]);
for (const auto& element : listedElements)
elements.append(element);
int invalidControlsCount = 0;
for (unsigned i = 0; i < elements.size(); ++i) {
if (elements[i]->form() == this && elements[i]->isFormControlElement()) {
HTMLFormControlElement* control =
toHTMLFormControlElement(elements[i].get());
for (const auto& element : elements) {
if (element->form() == this && element->isFormControlElement()) {
HTMLFormControlElement* control = toHTMLFormControlElement(element);
if (control->isSubmittableElement() &&
!control->checkValidity(unhandledInvalidControls, eventBehavior) &&
control->formOwner() == this) {
Expand Down
8 changes: 3 additions & 5 deletions third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1230,11 +1230,9 @@ bool HTMLMediaElement::textTracksAreReady() const {
// mode was not in the disabled state when the element's resource selection
// algorithm last started now have a text track readiness state of loaded or
// failed to load.
for (unsigned i = 0; i < m_textTracksWhenResourceSelectionBegan.size(); ++i) {
if (m_textTracksWhenResourceSelectionBegan[i]->getReadinessState() ==
TextTrack::Loading ||
m_textTracksWhenResourceSelectionBegan[i]->getReadinessState() ==
TextTrack::NotLoaded)
for (const auto& textTrack : m_textTracksWhenResourceSelectionBegan) {
if (textTrack->getReadinessState() == TextTrack::Loading ||
textTrack->getReadinessState() == TextTrack::NotLoaded)
return false;
}

Expand Down
5 changes: 2 additions & 3 deletions third_party/WebKit/Source/core/html/HTMLTextAreaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -480,9 +480,8 @@ void HTMLTextAreaElement::setDefaultValue(const String& defaultValue) {
if (n->isTextNode())
textNodes.append(n);
}
size_t size = textNodes.size();
for (size_t i = 0; i < size; ++i)
removeChild(textNodes[i].get(), IGNORE_EXCEPTION);
for (const auto& text : textNodes)
removeChild(text.get(), IGNORE_EXCEPTION);

// Normalize line endings.
String value = defaultValue;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,9 +150,7 @@ void MediaFragmentURIParser::parseTimeFragment() {

m_timeFormat = Invalid;

for (unsigned i = 0; i < m_fragments.size(); ++i) {
std::pair<String, String>& fragment = m_fragments[i];

for (const auto& fragment : m_fragments) {
DCHECK(fragment.first.is8Bit());
DCHECK(fragment.second.is8Bit());

Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/core/html/PublicURLManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ void PublicURLManager::revoke(const String& uuid) {
urlsToRemove.append(registeredUrl.key);
}
}
for (unsigned j = 0; j < urlsToRemove.size(); j++)
registeredURLs.remove(urlsToRemove[j]);
for (const auto& url : urlsToRemove)
registeredURLs.remove(url);
urlsToRemove.clear();
}
}
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/core/html/forms/EmailInputType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ String EmailInputType::findInvalidAddress(const String& value) const {
return isValidEmailAddress(ensureEmailRegexp(), value) ? String() : value;
Vector<String> addresses;
value.split(',', true, addresses);
for (unsigned i = 0; i < addresses.size(); ++i) {
String stripped = stripLeadingAndTrailingHTMLSpaces(addresses[i]);
for (const auto& address : addresses) {
String stripped = stripLeadingAndTrailingHTMLSpaces(address);
if (!isValidEmailAddress(ensureEmailRegexp(), stripped))
return stripped;
}
Expand Down
20 changes: 9 additions & 11 deletions third_party/WebKit/Source/core/html/forms/FileInputType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,24 +225,22 @@ FileList* FileInputType::createFileList(
int rootLength = rootPath.length();
if (rootPath[rootLength - 1] != '\\' && rootPath[rootLength - 1] != '/')
rootLength += 1;
for (size_t i = 0; i < size; ++i) {
for (const auto& file : files) {
// Normalize backslashes to slashes before exposing the relative path to
// script.
String relativePath =
files[i].path.substring(rootLength).replace('\\', '/');
fileList->append(
File::createWithRelativePath(files[i].path, relativePath));
String relativePath = file.path.substring(rootLength).replace('\\', '/');
fileList->append(File::createWithRelativePath(file.path, relativePath));
}
return fileList;
}

for (size_t i = 0; i < size; ++i) {
if (files[i].fileSystemURL.isEmpty()) {
for (const auto& file : files) {
if (file.fileSystemURL.isEmpty()) {
fileList->append(
File::createForUserProvidedFile(files[i].path, files[i].displayName));
File::createForUserProvidedFile(file.path, file.displayName));
} else {
fileList->append(File::createForFileSystemFile(
files[i].fileSystemURL, files[i].metadata, File::IsUserVisible));
file.fileSystemURL, file.metadata, File::IsUserVisible));
}
}
return fileList;
Expand Down Expand Up @@ -352,8 +350,8 @@ void FileInputType::setFilesFromPaths(const Vector<String>& paths) {
}

Vector<FileChooserFileInfo> files;
for (unsigned i = 0; i < paths.size(); ++i)
files.append(FileChooserFileInfo(paths[i]));
for (const auto& path : paths)
files.append(FileChooserFileInfo(path));

if (input.fastHasAttribute(multipleAttr)) {
filesChosen(files);
Expand Down
8 changes: 4 additions & 4 deletions third_party/WebKit/Source/core/html/forms/FormController.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ static inline HTMLFormElement* ownerFormForState(
void FormControlState::serializeTo(Vector<String>& stateVector) const {
DCHECK(!isFailure());
stateVector.append(String::number(m_values.size()));
for (size_t i = 0; i < m_values.size(); ++i)
stateVector.append(m_values[i].isNull() ? emptyString() : m_values[i]);
for (const auto& value : m_values)
stateVector.append(value.isNull() ? emptyString() : value);
}

FormControlState FormControlState::deserialize(
Expand Down Expand Up @@ -295,8 +295,8 @@ Vector<String> SavedFormState::getReferencedFilePaths() const {
const Vector<FileChooserFileInfo>& selectedFiles =
HTMLInputElement::filesFromFileInputFormControlState(
formControlState);
for (size_t i = 0; i < selectedFiles.size(); ++i)
toReturn.append(selectedFiles[i].path);
for (const auto& file : selectedFiles)
toReturn.append(file.path);
}
}
return toReturn;
Expand Down
4 changes: 2 additions & 2 deletions third_party/WebKit/Source/core/html/imports/HTMLImport.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ void HTMLImport::recalcTreeState(HTMLImport* root) {
updated.append(i);
}

for (size_t i = 0; i < updated.size(); ++i)
updated[i]->stateDidChange();
for (const auto& import : updated)
import->stateDidChange();
}

#if !defined(NDEBUG)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ bool HTMLImportLoader::hasPendingResources() const {
}

void HTMLImportLoader::didFinishLoading() {
for (size_t i = 0; i < m_imports.size(); ++i)
m_imports[i]->didFinishLoading();
for (const auto& importChild : m_imports)
importChild->didFinishLoading();

clearResource();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ HTMLImportTreeRoot::HTMLImportTreeRoot(Document* document)
HTMLImportTreeRoot::~HTMLImportTreeRoot() {}

void HTMLImportTreeRoot::dispose() {
for (size_t i = 0; i < m_imports.size(); ++i)
m_imports[i]->dispose();
for (const auto& importChild : m_imports)
importChild->dispose();
m_imports.clear();
m_document = nullptr;
m_recalcTimer.stop();
Expand Down Expand Up @@ -67,8 +67,7 @@ HTMLImportChild* HTMLImportTreeRoot::add(HTMLImportChild* child) {
}

HTMLImportChild* HTMLImportTreeRoot::find(const KURL& url) const {
for (size_t i = 0; i < m_imports.size(); ++i) {
HTMLImportChild* candidate = m_imports[i].get();
for (const auto& candidate : m_imports) {
if (equalIgnoringFragmentIdentifier(candidate->url(), url))
return candidate;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ void HTMLImportsController::dispose() {
m_root.clear();
}

for (size_t i = 0; i < m_loaders.size(); ++i)
m_loaders[i]->dispose();
for (const auto& loader : m_loaders)
loader->dispose();
m_loaders.clear();
}

Expand Down Expand Up @@ -139,9 +139,9 @@ HTMLImportLoader* HTMLImportsController::createLoader() {

HTMLImportLoader* HTMLImportsController::loaderFor(
const Document& document) const {
for (size_t i = 0; i < m_loaders.size(); ++i) {
if (m_loaders[i]->document() == &document)
return m_loaders[i].get();
for (const auto& loader : m_loaders) {
if (loader->document() == &document)
return loader.get();
}

return nullptr;
Expand Down
3 changes: 1 addition & 2 deletions third_party/WebKit/Source/core/html/parser/AtomicHTMLToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,7 @@ inline void AtomicHTMLToken::initializeAttributes(

m_attributes.clear();
m_attributes.reserveInitialCapacity(size);
for (size_t i = 0; i < size; ++i) {
const HTMLToken::Attribute& attribute = attributes[i];
for (const auto& attribute : attributes) {
if (attribute.nameAsVector().isEmpty())
continue;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,10 @@ void BackgroundHTMLInputStream::rewindTo(HTMLInputCheckpoint checkpointIndex,

void BackgroundHTMLInputStream::updateTotalCheckpointTokenCount() {
m_totalCheckpointTokenCount = 0;
size_t lastCheckpointIndex = m_checkpoints.size();
for (size_t i = 0; i < lastCheckpointIndex; ++i)
for (const auto& checkpoint : m_checkpoints) {
m_totalCheckpointTokenCount +=
m_checkpoints[i].tokensExtractedSincePreviousCheckpoint;
checkpoint.tokensExtractedSincePreviousCheckpoint;
}
}

} // namespace blink
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ void HTMLConstructionSite::executeQueuedTasks() {
TaskQueue queue;
queue.swap(m_taskQueue);

for (size_t i = 0; i < size; ++i)
executeTask(queue[i]);
for (auto& task : queue)
executeTask(task);

// We might be detached now.
}
Expand Down Expand Up @@ -407,8 +407,7 @@ void HTMLConstructionSite::mergeAttributesFromTokenIntoElement(
if (token->attributes().isEmpty())
return;

for (unsigned i = 0; i < token->attributes().size(); ++i) {
const Attribute& tokenAttribute = token->attributes().at(i);
for (const auto& tokenAttribute : token->attributes()) {
if (element->attributesWithoutUpdate().findIndex(tokenAttribute.name()) ==
kNotFound)
element->setAttribute(tokenAttribute.name(), tokenAttribute.value());
Expand Down
Loading

0 comments on commit f5775c2

Please sign in to comment.