Skip to content

Commit

Permalink
Proof-read comments in src/url/.
Browse files Browse the repository at this point in the history
Review URL: https://codereview.chromium.org/1270443006

Cr-Commit-Position: refs/heads/master@{#343473}
  • Loading branch information
qyearsley authored and Commit bot committed Aug 14, 2015
1 parent 1c92d5b commit 2bc727d
Show file tree
Hide file tree
Showing 25 changed files with 148 additions and 145 deletions.
17 changes: 9 additions & 8 deletions url/gurl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ const std::string& EmptyStringForGURL() {

#endif // WIN32

} // namespace
} // namespace

GURL::GURL() : is_valid_(false) {
}
Expand Down Expand Up @@ -132,7 +132,7 @@ void GURL::InitializeFromCanonicalSpec() {
#ifndef NDEBUG
// For testing purposes, check that the parsed canonical URL is identical to
// what we would have produced. Skip checking for invalid URLs have no meaning
// and we can't always canonicalize then reproducabely.
// and we can't always canonicalize then reproducibly.
if (is_valid_) {
url::Component scheme;
// We can't do this check on the inner_url of a filesystem URL, as
Expand Down Expand Up @@ -311,7 +311,7 @@ GURL GURL::ReplaceComponents(

GURL GURL::GetOrigin() const {
// This doesn't make sense for invalid or nonstandard URLs, so return
// the empty URL
// the empty URL.
if (!is_valid_ || !IsStandard())
return GURL();

Expand Down Expand Up @@ -408,16 +408,17 @@ std::string GURL::ExtractFileName() const {
}

std::string GURL::PathForRequest() const {
DCHECK(parsed_.path.len > 0) << "Canonical path for requests should be non-empty";
DCHECK(parsed_.path.len > 0)
<< "Canonical path for requests should be non-empty";
if (parsed_.ref.len >= 0) {
// Clip off the reference when it exists. The reference starts after the #
// sign, so we have to subtract one to also remove it.
// Clip off the reference when it exists. The reference starts after the
// #-sign, so we have to subtract one to also remove it.
return std::string(spec_, parsed_.path.begin,
parsed_.ref.begin - parsed_.path.begin - 1);
}
// Compute the actual path length, rather than depending on the spec's
// terminator. If we're an inner_url, our spec continues on into our outer
// url's path/query/ref.
// terminator. If we're an inner_url, our spec continues on into our outer
// URL's path/query/ref.
int path_len = parsed_.path.len;
if (parsed_.query.is_valid())
path_len = parsed_.query.end() - parsed_.path.begin;
Expand Down
19 changes: 9 additions & 10 deletions url/gurl.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ class URL_EXPORT GURL {

// Returns the potentially invalid spec for a the URL. This spec MUST NOT be
// modified or sent over the network. It is designed to be displayed in error
// messages to the user, as the apperance of the spec may explain the error.
// messages to the user, as the appearance of the spec may explain the error.
// If the spec is valid, the valid spec will be returned.
//
// The returned string is guaranteed to be valid UTF-8.
Expand Down Expand Up @@ -125,9 +125,8 @@ class URL_EXPORT GURL {
// pages.
//
// It may be impossible to resolve the URLs properly. If the input is not
// "standard" (SchemeIsStandard() == false) and the input looks relative, we
// can't resolve it. In these cases, the result will be an empty, invalid
// GURL.
// "standard" (IsStandard() == false) and the input looks relative, we can't
// resolve it. In these cases, the result will be an empty, invalid GURL.
//
// The result may also be a nonempty, invalid URL if the input has some kind
// of encoding error. In these cases, we will try to construct a "good" URL
Expand Down Expand Up @@ -283,8 +282,8 @@ class URL_EXPORT GURL {
return ComponentString(parsed_.ref);
}

// Existance querying. These functions will return true if the corresponding
// URL component exists in this URL. Note that existance is different than
// Existence querying. These functions will return true if the corresponding
// URL component exists in this URL. Note that existence is different than
// being nonempty. http://www.google.com/? has a query that just happens to
// be empty, and has_query() will return true.
bool has_scheme() const {
Expand All @@ -297,7 +296,7 @@ class URL_EXPORT GURL {
return parsed_.password.len >= 0;
}
bool has_host() const {
// Note that hosts are special, absense of host means length 0.
// Note that hosts are special, absence of host means length 0.
return parsed_.host.len > 0;
}
bool has_port() const {
Expand Down Expand Up @@ -347,7 +346,7 @@ class URL_EXPORT GURL {
// object constructions are done.
bool DomainIs(base::StringPiece lower_ascii_domain) const;

// Swaps the contents of this GURL object with the argument without doing
// Swaps the contents of this GURL object with |other|, without doing
// any memory allocations.
void Swap(GURL* other);

Expand All @@ -364,8 +363,8 @@ class URL_EXPORT GURL {

private:
// Variant of the string parsing constructor that allows the caller to elect
// retain trailing whitespace, if any, on the passed URL spec but only if the
// scheme is one that allows trailing whitespace. The primary use-case is
// retain trailing whitespace, if any, on the passed URL spec, but only if
// the scheme is one that allows trailing whitespace. The primary use-case is
// for data: URLs. In most cases, you want to use the single parameter
// constructor above.
enum RetainWhiteSpaceSelector { RETAIN_TRAILING_PATH_WHITEPACE };
Expand Down
21 changes: 11 additions & 10 deletions url/gurl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,23 @@ TEST(GURLTest, Types) {
EXPECT_EQ("something:///HOSTNAME.com/",
TypesTestCase("something:///HOSTNAME.com/"));

// In the reverse, known schemes should always trigger standard URL handling.
// Conversely, URLs with known schemes should always trigger standard URL
// handling.
EXPECT_EQ("http://hostname.com/", TypesTestCase("http:HOSTNAME.com"));
EXPECT_EQ("http://hostname.com/", TypesTestCase("http:/HOSTNAME.com"));
EXPECT_EQ("http://hostname.com/", TypesTestCase("http://HOSTNAME.com"));
EXPECT_EQ("http://hostname.com/", TypesTestCase("http:///HOSTNAME.com"));

#ifdef WIN32
// URLs that look like absolute Windows drive specs.
// URLs that look like Windows absolute path specs.
EXPECT_EQ("file:///C:/foo.txt", TypesTestCase("c:\\foo.txt"));
EXPECT_EQ("file:///Z:/foo.txt", TypesTestCase("Z|foo.txt"));
EXPECT_EQ("file://server/foo.txt", TypesTestCase("\\\\server\\foo.txt"));
EXPECT_EQ("file://server/foo.txt", TypesTestCase("//server/foo.txt"));
#endif
}

// Test the basic creation and querying of components in a GURL. We assume
// Test the basic creation and querying of components in a GURL. We assume that
// the parser is already tested and works, so we are mostly interested if the
// object does the right thing with the results.
TEST(GURLTest, Components) {
Expand Down Expand Up @@ -175,7 +176,7 @@ TEST(GURLTest, Assign) {
EXPECT_EQ("", invalid2.ref());
}

// This is a regression test for http://crbug.com/309975 .
// This is a regression test for http://crbug.com/309975.
TEST(GURLTest, SelfAssign) {
GURL a("filesystem:http://example.com/temporary/");
// This should not crash.
Expand Down Expand Up @@ -245,9 +246,9 @@ TEST(GURLTest, IsValid) {
}

TEST(GURLTest, ExtraSlashesBeforeAuthority) {
// According to RFC3986, the hier-part for URI with an authority must use only
// two slashes, GURL intentionally just ignores slashes more than 2 and parses
// the following part as an authority.
// According to RFC3986, the hierarchical part for URI with an authority
// must use only two slashes; GURL intentionally just ignores extra slashes
// if there are more than 2, and parses the following part as an authority.
GURL url("http:///host");
EXPECT_EQ("host", url.host());
EXPECT_EQ("/", url.path());
Expand Down Expand Up @@ -378,7 +379,7 @@ TEST(GURLTest, GetWithEmptyPath) {
}

TEST(GURLTest, Replacements) {
// The url canonicalizer replacement test will handle most of these case.
// The URL canonicalizer replacement test will handle most of these case.
// The most important thing to do here is to check that the proper
// canonicalizer gets called based on the scheme of the input.
struct ReplaceCase {
Expand All @@ -395,7 +396,7 @@ TEST(GURLTest, Replacements) {
} replace_cases[] = {
{"http://www.google.com/foo/bar.html?foo#bar", NULL, NULL, NULL, NULL, NULL, "/", "", "", "http://www.google.com/"},
{"http://www.google.com/foo/bar.html?foo#bar", "javascript", "", "", "", "", "window.open('foo');", "", "", "javascript:window.open('foo');"},
{"file:///C:/foo/bar.txt", "http", NULL, NULL, "www.google.com", "99", "/foo","search", "ref", "http://www.google.com:99/foo?search#ref"},
{"file:///C:/foo/bar.txt", "http", NULL, NULL, "www.google.com", "99", "/foo", "search", "ref", "http://www.google.com:99/foo?search#ref"},
#ifdef WIN32
{"http://www.google.com/foo/bar.html?foo#bar", "file", "", "", "", "", "c:\\", "", "", "file:///C:/"},
#endif
Expand Down Expand Up @@ -435,7 +436,7 @@ TEST(GURLTest, ClearFragmentOnDataUrl) {

EXPECT_EQ("data: one ? two ", url_no_ref.spec());

// Importing a parsed url via this constructor overload will retain trailing
// Importing a parsed URL via this constructor overload will retain trailing
// whitespace.
GURL import_url(url_no_ref.spec(),
url_no_ref.parsed_for_possibly_invalid_spec(),
Expand Down
2 changes: 1 addition & 1 deletion url/origin.h
Original file line number Diff line number Diff line change
Expand Up @@ -133,4 +133,4 @@ URL_EXPORT std::ostream& operator<<(std::ostream& out,

} // namespace url

#endif // URL_SCHEME_HOST_PORT_H_
#endif // URL_ORIGIN_H_
26 changes: 13 additions & 13 deletions url/url_canon.h
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ URL_EXPORT bool CanonicalizeScheme(const base::char16* spec,
// User info: username/password. If present, this will add the delimiters so
// the output will be "<username>:<password>@" or "<username>@". Empty
// username/password pairs, or empty passwords, will get converted to
// nonexistant in the canonical version.
// nonexistent in the canonical version.
//
// The components for the username and password refer to ranges in the
// respective source strings. Usually, these will be the same string, which
Expand Down Expand Up @@ -317,21 +317,21 @@ struct CanonHostInfo {

// This field summarizes how the input was classified by the canonicalizer.
enum Family {
NEUTRAL, // - Doesn't resemble an IP address. As far as the IP
NEUTRAL, // - Doesn't resemble an IP address. As far as the IP
// canonicalizer is concerned, it should be treated as a
// hostname.
BROKEN, // - Almost an IP, but was not canonicalized. This could be an
BROKEN, // - Almost an IP, but was not canonicalized. This could be an
// IPv4 address where truncation occurred, or something
// containing the special characters :[] which did not parse
// as an IPv6 address. Never attempt to connect to this
// as an IPv6 address. Never attempt to connect to this
// address, because it might actually succeed!
IPV4, // - Successfully canonicalized as an IPv4 address.
IPV6, // - Successfully canonicalized as an IPv6 address.
};
Family family;

// If |family| is IPV4, then this is the number of nonempty dot-separated
// components in the input text, from 1 to 4. If |family| is not IPV4,
// components in the input text, from 1 to 4. If |family| is not IPV4,
// this value is undefined.
int num_ipv4_components;

Expand All @@ -355,7 +355,7 @@ struct CanonHostInfo {

// Host.
//
// The 8-bit version requires UTF-8 encoding. Use this version when you only
// The 8-bit version requires UTF-8 encoding. Use this version when you only
// need to know whether canonicalization succeeded.
URL_EXPORT bool CanonicalizeHost(const char* spec,
const Component& host,
Expand All @@ -368,7 +368,7 @@ URL_EXPORT bool CanonicalizeHost(const base::char16* spec,

// Extended version of CanonicalizeHost, which returns additional information.
// Use this when you need to know whether the hostname was an IP address.
// A successful return is indicated by host_info->family != BROKEN. See the
// A successful return is indicated by host_info->family != BROKEN. See the
// definition of CanonHostInfo above for details.
URL_EXPORT void CanonicalizeHostVerbose(const char* spec,
const Component& host,
Expand Down Expand Up @@ -554,7 +554,7 @@ URL_EXPORT bool CanonicalizePathURL(const base::char16* spec,
CanonOutput* output,
Parsed* new_parsed);

// Use for mailto URLs. This "canonicalizes" the url into a path and query
// Use for mailto URLs. This "canonicalizes" the URL into a path and query
// component. It does not attempt to merge "to" fields. It uses UTF-8 for
// the query encoding if there is a query. This is because a mailto URL is
// really intended for an external mail program, and the encoding of a page,
Expand All @@ -578,9 +578,9 @@ URL_EXPORT bool CanonicalizeMailtoURL(const base::char16* spec,
// treated on the same code path as regular canonicalization (the same string
// for each component).
//
// A Parsed structure usually goes along with this. Those
// components identify offsets within these strings, so that they can all be
// in the same string, or spread arbitrarily across different ones.
// A Parsed structure usually goes along with this. Those components identify
// offsets within these strings, so that they can all be in the same string,
// or spread arbitrarily across different ones.
//
// This structures does not own any data. It is the caller's responsibility to
// ensure that the data the pointers point to stays in scope and is not
Expand Down Expand Up @@ -725,7 +725,7 @@ class Replacements {
}
bool IsRefOverridden() const { return sources_.ref != NULL; }

// Getters for the itnernal data. See the variables below for how the
// Getters for the internal data. See the variables below for how the
// information is encoded.
const URLComponentSource<CHAR>& sources() const { return sources_; }
const Parsed& components() const { return components_; }
Expand Down Expand Up @@ -863,7 +863,7 @@ URL_EXPORT bool IsRelativeURL(const char* base,
// The base URL should be canonical and have a host (may be empty for file
// URLs) and a path. If it doesn't have these, we can't resolve relative
// URLs off of it and will return the base as the output with an error flag.
// Becausee it is canonical is should also be ASCII.
// Because it is canonical is should also be ASCII.
//
// The query charset converter follows the same rules as CanonicalizeQuery.
//
Expand Down
8 changes: 4 additions & 4 deletions url/url_canon_etc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ bool DoScheme(const CHAR* spec,
// The output scheme starts from the current position.
out_scheme->begin = output->length();

// Danger: it's important that this code does not strip any characters: it
// only emits the canonical version (be it valid or escaped) of each of
// the input characters. Stripping would put it out of sync with
// Danger: it's important that this code does not strip any characters;
// it only emits the canonical version (be it valid or escaped) for each
// of the input characters. Stripping would put it out of sync with
// FindAndCompareScheme, which could cause some security checks on
// schemes to be incorrect.
bool success = true;
Expand Down Expand Up @@ -218,7 +218,7 @@ bool DoPort(const CHAR* spec,
char buf[buf_size];
WritePortInt(buf, buf_size, port_num);

// Append the port number to the output, preceeded by a colon.
// Append the port number to the output, preceded by a colon.
output->push_back(':');
out_port->begin = output->length();
for (int i = 0; i < buf_size && buf[i]; i++)
Expand Down
8 changes: 4 additions & 4 deletions url/url_canon_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace {
// NOTE: I didn't actually test all the control characters. Some may be
// disallowed in the input, but they are all accepted escaped except for 0.
// I also didn't test if characters affecting HTML parsing are allowed
// unescaped, eg. (") or (#), which would indicate the beginning of the path.
// unescaped, e.g. (") or (#), which would indicate the beginning of the path.
// Surprisingly, space is accepted in the input and always escaped.

// This table lists the canonical version of all characters we allow in the
Expand Down Expand Up @@ -316,19 +316,19 @@ void DoHost(const CHAR* spec,
}

if (!success) {
// Canonicalization failed. Set BROKEN to notify the caller.
// Canonicalization failed. Set BROKEN to notify the caller.
host_info->family = CanonHostInfo::BROKEN;
} else {
// After all the other canonicalization, check if we ended up with an IP
// address. IP addresses are small, so writing into this temporary buffer
// address. IP addresses are small, so writing into this temporary buffer
// should not cause an allocation.
RawCanonOutput<64> canon_ip;
CanonicalizeIPAddress(output->data(),
MakeRange(output_begin, output->length()),
&canon_ip, host_info);

// If we got an IPv4/IPv6 address, copy the canonical form back to the
// real buffer. Otherwise, it's a hostname or broken IP, in which case
// real buffer. Otherwise, it's a hostname or broken IP, in which case
// we just leave it in place.
if (host_info->IsIPAddress()) {
output->set_length(output_begin);
Expand Down
8 changes: 4 additions & 4 deletions url/url_canon_internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,9 @@ void AppendStringOfType(const base::char16* source, int length,

bool ReadUTFChar(const char* str, int* begin, int length,
unsigned* code_point_out) {
// This depends on ints and int32s being the same thing. If they're not, it
// This depends on ints and int32s being the same thing. If they're not, it
// will fail to compile.
// TODO(mmenke): This should probably be fixed.
// TODO(mmenke): This should probably be fixed.
if (!base::ReadUnicodeCharacter(str, length, begin, code_point_out) ||
!base::IsValidCharacter(*code_point_out)) {
*code_point_out = kUnicodeReplacementCharacter;
Expand All @@ -262,9 +262,9 @@ bool ReadUTFChar(const char* str, int* begin, int length,

bool ReadUTFChar(const base::char16* str, int* begin, int length,
unsigned* code_point_out) {
// This depends on ints and int32s being the same thing. If they're not, it
// This depends on ints and int32s being the same thing. If they're not, it
// will fail to compile.
// TODO(mmenke): This should probably be fixed.
// TODO(mmenke): This should probably be fixed.
if (!base::ReadUnicodeCharacter(str, length, begin, code_point_out) ||
!base::IsValidCharacter(*code_point_out)) {
*code_point_out = kUnicodeReplacementCharacter;
Expand Down
Loading

0 comments on commit 2bc727d

Please sign in to comment.