Skip to content

Commit

Permalink
Add std::string constructors for Origin/SchemeHostPort to reduce copies
Browse files Browse the repository at this point in the history
WebSecurityOrigin forwards temporaries to Origin. We should just use
these, rather than consider them opaque StringPieces and copying data
out of them.

This CL also removes some dead code.

BUG=672877

Review-Url: https://codereview.chromium.org/2561363002
Cr-Commit-Position: refs/heads/master@{#438054}
  • Loading branch information
csharrison authored and Commit bot committed Dec 13, 2016
1 parent 38f4a83 commit f07ac3c
Show file tree
Hide file tree
Showing 6 changed files with 38 additions and 41 deletions.
8 changes: 5 additions & 3 deletions content/child/blink_platform_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ void CheckCastedOriginsAlreadyNormalized(
url::Origin::UnsafelyCreateOriginWithoutNormalization(
origin.protocol().utf8(), origin.host().utf8(),
origin.effectivePort());
url::Origin non_checked_origin = url::Origin::CreateFromNormalizedTuple(
origin.protocol().utf8(), origin.host().utf8(), origin.effectivePort());
url::Origin non_checked_origin =
url::Origin::CreateFromNormalizedTupleWithSuborigin(
origin.protocol().utf8(), origin.host().utf8(),
origin.effectivePort(), origin.suborigin().utf8());
EXPECT_EQ(checked_origin.scheme(), non_checked_origin.scheme());
EXPECT_EQ(checked_origin.host(), non_checked_origin.host());
EXPECT_EQ(checked_origin.port(), non_checked_origin.port());
Expand Down Expand Up @@ -144,7 +146,7 @@ TEST(BlinkPlatformTest, CastWebSecurityOrigin) {
}

// This test ensures that WebSecurityOrigins can safely use
// url::Origin::CreateFromNormalizedTuple when doing conversions.
// url::Origin::CreateFromNormalizedTupleWithSuborigin when doing conversions.
TEST(BlinkPlatformTest, WebSecurityOriginNormalization) {
struct TestCases {
const char* url;
Expand Down
26 changes: 15 additions & 11 deletions url/origin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,21 @@ Origin::Origin(base::StringPiece scheme,
uint16_t port,
base::StringPiece suborigin,
SchemeHostPort::ConstructPolicy policy)
: tuple_(scheme, host, port, policy) {
: tuple_(scheme.as_string(), host.as_string(), port, policy) {
unique_ = tuple_.IsInvalid();
suborigin_ = suborigin.as_string();
}

Origin::Origin(std::string scheme,
std::string host,
uint16_t port,
std::string suborigin,
SchemeHostPort::ConstructPolicy policy)
: tuple_(std::move(scheme), std::move(host), port, policy) {
unique_ = tuple_.IsInvalid();
suborigin_ = std::move(suborigin);
}

Origin::~Origin() {
}

Expand All @@ -101,18 +111,12 @@ Origin Origin::UnsafelyCreateOriginWithoutNormalization(
return Origin(scheme, host, port, "", SchemeHostPort::CHECK_CANONICALIZATION);
}

Origin Origin::CreateFromNormalizedTuple(base::StringPiece scheme,
base::StringPiece host,
uint16_t port) {
return CreateFromNormalizedTupleWithSuborigin(scheme, host, port, "");
}

Origin Origin::CreateFromNormalizedTupleWithSuborigin(
base::StringPiece scheme,
base::StringPiece host,
std::string scheme,
std::string host,
uint16_t port,
base::StringPiece suborigin) {
return Origin(scheme, host, port, suborigin,
std::string suborigin) {
return Origin(std::move(scheme), std::move(host), port, std::move(suborigin),
SchemeHostPort::ALREADY_CANONICALIZED);
}

Expand Down
20 changes: 10 additions & 10 deletions url/origin.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,18 +104,13 @@ class URL_EXPORT Origin {

// Creates an origin without sanity checking that the host is canonicalized.
// This should only be used when converting between already normalized types,
// and should NOT be used for IPC.
static Origin CreateFromNormalizedTuple(base::StringPiece scheme,
base::StringPiece host,
uint16_t port);

// Same as CreateFromNormalizedTuple() above, but adds a suborigin component
// as well.
// and should NOT be used for IPC. Method takes std::strings for use with move
// operators to avoid copies.
static Origin CreateFromNormalizedTupleWithSuborigin(
base::StringPiece scheme,
base::StringPiece host,
std::string scheme,
std::string host,
uint16_t port,
base::StringPiece suborigin);
std::string suborigin);

~Origin();

Expand Down Expand Up @@ -173,6 +168,11 @@ class URL_EXPORT Origin {
uint16_t port,
base::StringPiece suborigin,
SchemeHostPort::ConstructPolicy policy);
Origin(std::string scheme,
std::string host,
uint16_t port,
std::string suborigin,
SchemeHostPort::ConstructPolicy policy);

SchemeHostPort tuple_;
bool unique_;
Expand Down
9 changes: 0 additions & 9 deletions url/origin_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,11 @@ TEST(OriginTest, ConstructFromTuple) {
<< test_case.port;
}
SCOPED_TRACE(scope_message);

url::Origin origin_without_suborigin =
url::Origin::CreateFromNormalizedTuple(test_case.scheme, test_case.host,
test_case.port);

url::Origin origin_with_suborigin =
url::Origin::CreateFromNormalizedTupleWithSuborigin(
test_case.scheme, test_case.host, test_case.port,
test_case.suborigin);

EXPECT_EQ(test_case.scheme, origin_without_suborigin.scheme());
EXPECT_EQ(test_case.host, origin_without_suborigin.host());
EXPECT_EQ(test_case.port, origin_without_suborigin.port());

EXPECT_EQ(test_case.scheme, origin_with_suborigin.scheme());
EXPECT_EQ(test_case.host, origin_with_suborigin.host());
EXPECT_EQ(test_case.port, origin_with_suborigin.port());
Expand Down
12 changes: 6 additions & 6 deletions url/scheme_host_port.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,24 +116,24 @@ bool IsValidInput(const base::StringPiece& scheme,
SchemeHostPort::SchemeHostPort() : port_(0) {
}

SchemeHostPort::SchemeHostPort(base::StringPiece scheme,
base::StringPiece host,
SchemeHostPort::SchemeHostPort(std::string scheme,
std::string host,
uint16_t port,
ConstructPolicy policy)
: port_(0) {
if (!IsValidInput(scheme, host, port, policy))
return;

scheme.CopyToString(&scheme_);
host.CopyToString(&host_);
scheme_ = std::move(scheme);
host_ = std::move(host);
port_ = port;
}

SchemeHostPort::SchemeHostPort(base::StringPiece scheme,
base::StringPiece host,
uint16_t port)
: SchemeHostPort(scheme,
host,
: SchemeHostPort(scheme.as_string(),
host.as_string(),
port,
ConstructPolicy::CHECK_CANONICALIZATION) {}

Expand Down
4 changes: 2 additions & 2 deletions url/scheme_host_port.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ class URL_EXPORT SchemeHostPort {
// that the host and port are canonicalized. This should only be used when
// converting between already normalized types, and should NOT be used for
// IPC.
SchemeHostPort(base::StringPiece scheme,
base::StringPiece host,
SchemeHostPort(std::string scheme,
std::string host,
uint16_t port,
ConstructPolicy policy);

Expand Down

0 comments on commit f07ac3c

Please sign in to comment.