From a586a4f1693ae618c191679d7c16a0de940db67d Mon Sep 17 00:00:00 2001 From: Matt Menke Date: Tue, 14 Jun 2016 14:27:13 -0400 Subject: [PATCH] Fix generation of inner_url in GURL::Replacements. 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 73cea7e4afe9d7ffc45cecc80da954a481bbb48d) Review URL: https://codereview.chromium.org/2062183002 . Cr-Commit-Position: refs/branch-heads/2743@{#352} Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939} --- url/gurl.cc | 6 ++++-- url/gurl.h | 4 ++++ url/gurl_unittest.cc | 27 ++++++++++++++++++++++----- 3 files changed, 30 insertions(+), 7 deletions(-) diff --git a/url/gurl.cc b/url/gurl.cc index 2b67beebc498d..2f8181329dd2e 100644 --- a/url/gurl.cc +++ b/url/gurl.cc @@ -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; @@ -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; diff --git a/url/gurl.h b/url/gurl.h index 103f1d8519ca1..8a1750fa5bb42 100644 --- a/url/gurl.h +++ b/url/gurl.h @@ -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(); } diff --git a/url/gurl_unittest.cc b/url/gurl_unittest.cc index abc3f10634766..4378db94547f4 100644 --- a/url/gurl_unittest.cc +++ b/url/gurl_unittest.cc @@ -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++) { @@ -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()); + } } }