Skip to content

Commit

Permalink
Fix generation of inner_url in GURL::Replacements.
Browse files Browse the repository at this point in the history
It was being created using a combination of the old and new URLs,
instead of just using the new URL.

BUG=615820

Review-Url: https://codereview.chromium.org/2029213002
Cr-Commit-Position: refs/heads/master@{#399501}
(cherry picked from commit 73cea7e)

Review URL: https://codereview.chromium.org/2062183002 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#352}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
  • Loading branch information
Matt Menke committed Jun 14, 2016
1 parent 964f4be commit a586a4f
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 7 deletions.
6 changes: 4 additions & 2 deletions url/gurl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,8 @@ GURL GURL::ReplaceComponents(

output.Complete();
if (result.is_valid_ && result.SchemeIsFileSystem()) {
result.inner_url_.reset(new GURL(spec_.data(), result.parsed_.Length(),
result.inner_url_.reset(new GURL(result.spec_.data(),
result.parsed_.Length(),
*result.parsed_.inner_parsed(), true));
}
return result;
Expand All @@ -306,7 +307,8 @@ GURL GURL::ReplaceComponents(

output.Complete();
if (result.is_valid_ && result.SchemeIsFileSystem()) {
result.inner_url_.reset(new GURL(spec_.data(), result.parsed_.Length(),
result.inner_url_.reset(new GURL(result.spec_.data(),
result.parsed_.Length(),
*result.parsed_.inner_parsed(), true));
}
return result;
Expand Down
4 changes: 4 additions & 0 deletions url/gurl.h
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,10 @@ class URL_EXPORT GURL {

// Returns the inner URL of a nested URL (currently only non-null for
// filesystem URLs).
//
// TODO(mmenke): inner_url().spec() currently returns the same value as
// caling spec() on the GURL itself. This should be fixed.
// See https://crbug.com/619596
const GURL* inner_url() const {
return inner_url_.get();
}
Expand Down
27 changes: 22 additions & 5 deletions url/gurl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -401,13 +401,23 @@ TEST(GURLTest, Replacements) {
const char* ref;
const char* expected;
} 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"},
{"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"},
#ifdef WIN32
{"http://www.google.com/foo/bar.html?foo#bar", "file", "", "", "", "", "c:\\", "", "", "file:///C:/"},
{"http://www.google.com/foo/bar.html?foo#bar", "file", "", "", "", "",
"c:\\", "", "", "file:///C:/"},
#endif
{"filesystem:http://www.google.com/foo/bar.html?foo#bar", NULL, NULL, NULL, NULL, NULL, "/", "", "", "filesystem:http://www.google.com/foo/"},
{"filesystem:http://www.google.com/foo/bar.html?foo#bar", NULL, NULL,
NULL, NULL, NULL, "/", "", "", "filesystem:http://www.google.com/foo/"},
// Lengthen the URL instead of shortening it, to test creation of
// inner_url.
{"filesystem:http://www.google.com/foo/", NULL, NULL, NULL, NULL, NULL,
"bar.html", "foo", "bar",
"filesystem:http://www.google.com/foo/bar.html?foo#bar"},
};

for (size_t i = 0; i < arraysize(replace_cases); i++) {
Expand All @@ -425,7 +435,14 @@ TEST(GURLTest, Replacements) {
GURL output = url.ReplaceComponents(repl);

EXPECT_EQ(replace_cases[i].expected, output.spec());

EXPECT_EQ(output.SchemeIsFileSystem(), output.inner_url() != NULL);
if (output.SchemeIsFileSystem()) {
// TODO(mmenke): inner_url()->spec() is currently the same as the spec()
// for the GURL itself. This should be fixed.
// See https://crbug.com/619596
EXPECT_EQ(replace_cases[i].expected, output.inner_url()->spec());
}
}
}

Expand Down

0 comments on commit a586a4f

Please sign in to comment.