Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

Commit

Permalink
Reduce the perf overhead of our dangling markup experiments.
Browse files Browse the repository at this point in the history
Given that Document::completeURL is a hotspot, this patch revamps the
approach to capture removed whitespace up in //url (where we're already
scanning through the string), and limits the scan for '<' to cases in
which we've actually removed whitespace.

BUG=680970, 682300
R=jochen@chromium.org

Review-Url: https://codereview.chromium.org/2643613002
Cr-Commit-Position: refs/heads/master@{#444734}
  • Loading branch information
mikewest authored and Commit bot committed Jan 19, 2017
1 parent 1723178 commit 41318f4
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 29 deletions.
35 changes: 14 additions & 21 deletions third_party/WebKit/Source/core/dom/Document.cpp
Expand Up @@ -4945,34 +4945,27 @@ void Document::setEncodingData(const DocumentEncodingData& newData) {
}

KURL Document::completeURL(const String& url) const {
String trimmed = url.stripWhiteSpace();
KURL completed = completeURLWithOverride(url, m_baseURL);

bool newline = trimmed.contains('\n') || trimmed.contains('\r');
bool lessThan = trimmed.contains('<');
if ((newline || lessThan) && completed.protocolIsInHTTPFamily()) {
if (newline) {
if (completed.whitespaceRemoved()) {
if (completed.protocolIsInHTTPFamily()) {
UseCounter::count(*this,
UseCounter::DocumentCompleteURLHTTPContainingNewline);
}
if (lessThan) {
UseCounter::count(*this,
UseCounter::DocumentCompleteURLHTTPContainingLessThan);
}
if (newline && lessThan) {
bool lessThan = url.contains('<');
if (lessThan) {
UseCounter::count(
*this,
UseCounter::DocumentCompleteURLHTTPContainingNewlineAndLessThan);

if (RuntimeEnabledFeatures::restrictCompleteURLCharacterSetEnabled())
return KURL();
}
} else {
UseCounter::count(
*this,
UseCounter::DocumentCompleteURLHTTPContainingNewlineAndLessThan);

if (RuntimeEnabledFeatures::restrictCompleteURLCharacterSetEnabled())
return KURL();
*this, UseCounter::DocumentCompleteURLNonHTTPContainingNewline);
}
} else if (newline || lessThan) {
UseCounter::count(
*this,
UseCounter::DocumentCompleteURLNonHTTPContainingNewlineOrLessThan);
}
return completeURLWithOverride(url, m_baseURL);
return completed;
}

KURL Document::completeURLWithOverride(const String& url,
Expand Down
3 changes: 1 addition & 2 deletions third_party/WebKit/Source/core/frame/UseCounter.h
Expand Up @@ -1427,9 +1427,8 @@ class CORE_EXPORT UseCounter {
V8PromiseConstructorReturnedUndefined = 1766,
FormSubmittedWithUnclosedFormControl = 1767,
DocumentCompleteURLHTTPContainingNewline = 1768,
DocumentCompleteURLHTTPContainingLessThan = 1769,
DocumentCompleteURLHTTPContainingNewlineAndLessThan = 1770,
DocumentCompleteURLNonHTTPContainingNewlineOrLessThan = 1771,
DocumentCompleteURLNonHTTPContainingNewline = 1771,
CSSSelectorInternalMediaControlsTextTrackList = 1772,
CSSSelectorInternalMediaControlsTextTrackListItem = 1773,
CSSSelectorInternalMediaControlsTextTrackListItemInput = 1774,
Expand Down
2 changes: 2 additions & 0 deletions third_party/WebKit/Source/platform/weborigin/KURL.h
Expand Up @@ -191,6 +191,8 @@ class PLATFORM_EXPORT KURL {

bool isSafeToSendToAnotherThread() const;

bool whitespaceRemoved() const { return m_parsed.whitespace_removed; }

private:
void init(const KURL& base,
const String& relative,
Expand Down
31 changes: 31 additions & 0 deletions third_party/WebKit/Source/platform/weborigin/KURLTest.cpp
Expand Up @@ -302,6 +302,37 @@ TEST(KURLTest, EncodeWithURLEscapeSequences) {
EXPECT_EQ(encodeWithURLEscapeSequences(precomposed), "%C4%99");
}

TEST(KURLTest, RemoveWhitespace) {
struct {
const char* input;
const char* expected;
} cases[] = {
{"ht\ntps://example.com/yay?boo#foo", "https://example.com/yay?boo#foo"},
{"ht\ttps://example.com/yay?boo#foo", "https://example.com/yay?boo#foo"},
{"ht\rtps://example.com/yay?boo#foo", "https://example.com/yay?boo#foo"},
{"https://exa\nmple.com/yay?boo#foo", "https://example.com/yay?boo#foo"},
{"https://exa\tmple.com/yay?boo#foo", "https://example.com/yay?boo#foo"},
{"https://exa\rmple.com/yay?boo#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/y\nay?boo#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/y\tay?boo#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/y\ray?boo#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/yay?b\noo#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/yay?b\too#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/yay?b\roo#foo", "https://example.com/yay?boo#foo"},
{"https://example.com/yay?boo#f\noo", "https://example.com/yay?boo#foo"},
{"https://example.com/yay?boo#f\too", "https://example.com/yay?boo#foo"},
{"https://example.com/yay?boo#f\roo", "https://example.com/yay?boo#foo"},
};

for (const auto& test : cases) {
const KURL input(ParsedURLString, test.input);
const KURL expected(ParsedURLString, test.expected);
EXPECT_EQ(input, expected);
EXPECT_TRUE(input.whitespaceRemoved());
EXPECT_FALSE(expected.whitespaceRemoved());
}
}

TEST(KURLTest, ResolveEmpty) {
KURL emptyBase;

Expand Down
3 changes: 1 addition & 2 deletions tools/metrics/histograms/histograms.xml
Expand Up @@ -89402,8 +89402,7 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
<int value="1769" label="DocumentCompleteURLHTTPContainingLessThan"/>
<int value="1770"
label="DocumentCompleteURLHTTPContainingNewlineAndLessThan"/>
<int value="1771"
label="DocumentCompleteURLNonHTTPContainingNewlineOrLessThan"/>
<int value="1771" label="DocumentCompleteURLNonHTTPContainingNewline"/>
<int value="1772" label="CSSSelectorInternalMediaControlsTextTrackList"/>
<int value="1773" label="CSSSelectorInternalMediaControlsTextTrackListItem"/>
<int value="1774"
Expand Down
3 changes: 1 addition & 2 deletions url/third_party/mozilla/url_parse.cc
Expand Up @@ -690,8 +690,7 @@ bool DoExtractQueryKeyValue(const CHAR* spec,

} // namespace

Parsed::Parsed() : inner_parsed_(NULL) {
}
Parsed::Parsed() : whitespace_removed(false), inner_parsed_(NULL) {}

Parsed::Parsed(const Parsed& other) :
scheme(other.scheme),
Expand Down
3 changes: 3 additions & 0 deletions url/third_party/mozilla/url_parse.h
Expand Up @@ -177,6 +177,9 @@ struct URL_EXPORT Parsed {
// the string with the scheme stripped off.
Component GetContent() const;

// True if whitespace was removed from the URL during parsing.
bool whitespace_removed;

// This is used for nested URL types, currently only filesystem. If you
// parse a filesystem URL, the resulting Parsed will have a nested
// inner_parsed_ to hold the parsed inner URL's component information.
Expand Down
12 changes: 10 additions & 2 deletions url/url_util.cc
Expand Up @@ -199,8 +199,13 @@ bool DoCanonicalize(const CHAR* spec,
// Remove any whitespace from the middle of the relative URL if necessary.
// Possibly this will result in copying to the new buffer.
RawCanonOutputT<CHAR> whitespace_buffer;
if (whitespace_policy == REMOVE_WHITESPACE)
spec = RemoveURLWhitespace(spec, spec_len, &whitespace_buffer, &spec_len);
if (whitespace_policy == REMOVE_WHITESPACE) {
int original_len = spec_len;
spec =
RemoveURLWhitespace(spec, original_len, &whitespace_buffer, &spec_len);
if (spec_len != original_len)
output_parsed->whitespace_removed = true;
}

Parsed parsed_input;
#ifdef WIN32
Expand Down Expand Up @@ -280,6 +285,9 @@ bool DoResolveRelative(const char* base_spec,
const CHAR* relative = RemoveURLWhitespace(in_relative, in_relative_length,
&whitespace_buffer,
&relative_length);
if (in_relative_length != relative_length)
output_parsed->whitespace_removed = true;

bool base_is_authority_based = false;
bool base_is_hierarchical = false;
if (base_spec &&
Expand Down

0 comments on commit 41318f4

Please sign in to comment.