Skip to content

Commit

Permalink
Allow url::SchemeHostPort to hold non-file scheme without port
Browse files Browse the repository at this point in the history
WebSockets use url::Origin to pass origin info between renderer and
browser. Currently, it cannot hold an origin with non-file scheme and
no port. Chrome extensions have been using such origins, so we need
to keep the channel to convey origin info work for such origins.

BUG=516971
R=sleevi,brettw

Committed: https://crrev.com/1ac9ec7bccd1b5178b18338b10149f36292f5fb6
Cr-Commit-Position: refs/heads/master@{#343895}

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

Cr-Commit-Position: refs/heads/master@{#344181}
  • Loading branch information
tyoshino authored and Commit bot committed Aug 19, 2015
1 parent 2dafe52 commit 11a7c9f
Show file tree
Hide file tree
Showing 20 changed files with 309 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ TEST(ChromeOSFileSystemBackendTest, GetRootDirectories) {
}

TEST(ChromeOSFileSystemBackendTest, AccessPermissions) {
url::AddStandardScheme("chrome-extension");
url::AddStandardScheme("chrome-extension", url::SCHEME_WITHOUT_PORT);

scoped_refptr<storage::ExternalMountPoints> mount_points(
storage::ExternalMountPoints::CreateRefCounted());
Expand Down
30 changes: 21 additions & 9 deletions chrome/common/chrome_content_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -506,21 +506,33 @@ void ChromeContentClient::AddPepperPlugins(
#endif // defined(ENABLE_PLUGINS)
}

#if defined(OS_CHROMEOS)
static const int kNumChromeStandardURLSchemes = 6;
#else
static const int kNumChromeStandardURLSchemes = 5;
#endif
static const url::SchemeWithType kChromeStandardURLSchemes[
kNumChromeStandardURLSchemes] = {
{extensions::kExtensionScheme, url::SCHEME_WITHOUT_PORT},
{chrome::kChromeNativeScheme, url::SCHEME_WITHOUT_PORT},
{extensions::kExtensionResourceScheme, url::SCHEME_WITHOUT_PORT},
{chrome::kChromeSearchScheme, url::SCHEME_WITHOUT_PORT},
{dom_distiller::kDomDistillerScheme, url::SCHEME_WITHOUT_PORT},
#if defined(OS_CHROMEOS)
{chrome::kCrosScheme, url::SCHEME_WITHOUT_PORT},
#endif
};

void ChromeContentClient::AddAdditionalSchemes(
std::vector<std::string>* standard_schemes,
std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* savable_schemes) {
standard_schemes->push_back(extensions::kExtensionScheme);
for (int i = 0; i < kNumChromeStandardURLSchemes; i++)
standard_schemes->push_back(kChromeStandardURLSchemes[i]);

savable_schemes->push_back(extensions::kExtensionScheme);
standard_schemes->push_back(chrome::kChromeNativeScheme);
standard_schemes->push_back(extensions::kExtensionResourceScheme);
savable_schemes->push_back(extensions::kExtensionResourceScheme);
standard_schemes->push_back(chrome::kChromeSearchScheme);
savable_schemes->push_back(chrome::kChromeSearchScheme);
standard_schemes->push_back(dom_distiller::kDomDistillerScheme);
savable_schemes->push_back(dom_distiller::kDomDistillerScheme);
#if defined(OS_CHROMEOS)
standard_schemes->push_back(chrome::kCrosScheme);
#endif
}

std::string ChromeContentClient::GetProduct() const {
Expand Down
4 changes: 3 additions & 1 deletion chrome/common/chrome_content_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
#include "content/public/common/pepper_plugin_info.h"
#endif

#include "url/url_util.h"

// Returns the user agent of Chrome.
std::string GetUserAgent();

Expand Down Expand Up @@ -54,7 +56,7 @@ class ChromeContentClient : public content::ContentClient {
void SetGpuInfo(const gpu::GPUInfo& gpu_info) override;
void AddPepperPlugins(
std::vector<content::PepperPluginInfo>* plugins) override;
void AddAdditionalSchemes(std::vector<std::string>* standard_schemes,
void AddAdditionalSchemes(std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* saveable_shemes) override;
std::string GetProduct() const override;
std::string GetUserAgent() const override;
Expand Down
3 changes: 2 additions & 1 deletion chrome/common/chrome_content_client_ios.mm
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "url/gurl.h"
#include "url/url_util.h"

// TODO(ios): Investigate merging with chrome_content_client.cc; this would
// requiring either a lot of ifdefing, or spliting the file into parts.
Expand All @@ -30,7 +31,7 @@
}

void ChromeContentClient::AddAdditionalSchemes(
std::vector<std::string>* standard_schemes,
std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* saveable_shemes) {
// No additional schemes for iOS.
}
Expand Down
18 changes: 18 additions & 0 deletions chrome/common/chrome_content_client_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@

#include "chrome/common/chrome_content_client.h"

#include <string.h>

#include "base/command_line.h"
#include "base/memory/scoped_ptr.h"
#include "base/memory/scoped_vector.h"
#include "base/strings/string_split.h"
#include "content/public/common/content_switches.h"
#include "extensions/common/constants.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
#include "url/origin.h"
#include "url/url_util.h"

namespace {

Expand Down Expand Up @@ -121,4 +127,16 @@ TEST(ChromeContentClientTest, FindMostRecent) {
}
#endif // defined(ENABLE_PLUGINS)

TEST(ChromeContentClientTest, AdditionalSchemes) {
EXPECT_TRUE(url::IsStandard(
extensions::kExtensionScheme,
url::Component(0, strlen(extensions::kExtensionScheme))));

GURL extension_url(
"chrome-extension://abcdefghijklmnopqrstuvwxyzabcdef/foo.html");
url::Origin origin(extension_url);
EXPECT_EQ("chrome-extension://abcdefghijklmnopqrstuvwxyzabcdef",
origin.Serialize());
}

} // namespace chrome_common
8 changes: 4 additions & 4 deletions components/test/run_all_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ class ComponentsTestSuite : public base::TestSuite {

// These schemes need to be added globally to pass tests of
// autocomplete_input_unittest.cc and content_settings_pattern*
url::AddStandardScheme("chrome");
url::AddStandardScheme("chrome-extension");
url::AddStandardScheme("chrome-devtools");
url::AddStandardScheme("chrome-search");
url::AddStandardScheme("chrome", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome-extension", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome-devtools", url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme("chrome-search", url::SCHEME_WITHOUT_PORT);

// Not using kExtensionScheme to avoid the dependency to extensions.
ContentSettingsPattern::SetNonWildcardDomainNonPortScheme(
Expand Down
4 changes: 2 additions & 2 deletions content/browser/site_instance_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,8 @@ class SiteInstanceTest : public testing::Test {

void SetUp() override {
old_browser_client_ = SetBrowserClientForTesting(&browser_client_);
url::AddStandardScheme(kPrivilegedScheme);
url::AddStandardScheme(kChromeUIScheme);
url::AddStandardScheme(kPrivilegedScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITHOUT_PORT);

SiteInstanceImpl::set_render_process_host_factory(&rph_factory_);
}
Expand Down
14 changes: 7 additions & 7 deletions content/common/url_schemes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,24 @@

namespace {

void AddStandardSchemeHelper(const std::string& scheme) {
url::AddStandardScheme(scheme.c_str());
void AddStandardSchemeHelper(const url::SchemeWithType& scheme) {
url::AddStandardScheme(scheme.scheme, scheme.type);
}

} // namespace

namespace content {

void RegisterContentSchemes(bool lock_standard_schemes) {
std::vector<std::string> additional_standard_schemes;
std::vector<url::SchemeWithType> additional_standard_schemes;
std::vector<std::string> additional_savable_schemes;
GetContentClient()->AddAdditionalSchemes(&additional_standard_schemes,
&additional_savable_schemes);

url::AddStandardScheme(kChromeDevToolsScheme);
url::AddStandardScheme(kChromeUIScheme);
url::AddStandardScheme(kGuestScheme);
url::AddStandardScheme(kMetadataScheme);
url::AddStandardScheme(kChromeDevToolsScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kChromeUIScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kGuestScheme, url::SCHEME_WITHOUT_PORT);
url::AddStandardScheme(kMetadataScheme, url::SCHEME_WITHOUT_AUTHORITY);
std::for_each(additional_standard_schemes.begin(),
additional_standard_schemes.end(),
AddStandardSchemeHelper);
Expand Down
3 changes: 2 additions & 1 deletion content/public/common/content_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "build/build_config.h"
#include "content/common/content_export.h"
#include "ui/base/layout.h"
#include "url/url_util.h"

class GURL;

Expand Down Expand Up @@ -89,7 +90,7 @@ class CONTENT_EXPORT ContentClient {
// Gives the embedder a chance to register its own standard and saveable
// url schemes early on in the startup sequence.
virtual void AddAdditionalSchemes(
std::vector<std::string>* standard_schemes,
std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* savable_schemes) {}

// Returns whether the given message should be sent in a swapped out renderer.
Expand Down
14 changes: 11 additions & 3 deletions extensions/shell/common/shell_content_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,20 @@ void ShellContentClient::AddPepperPlugins(
#endif // !defined(DISABLE_NACL)
}

static const int kNumShellStandardURLSchemes = 2;
static const url::SchemeWithType kShellStandardURLSchemes[
kNumShellStandardURLSchemes] = {
{extensions::kExtensionScheme, url::SCHEME_WITHOUT_PORT},
{extensions::kExtensionResourceScheme, url::SCHEME_WITHOUT_PORT},
};

void ShellContentClient::AddAdditionalSchemes(
std::vector<std::string>* standard_schemes,
std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* savable_schemes) {
standard_schemes->push_back(kExtensionScheme);
for (int i = 0; i < kNumShellStandardURLSchemes; i++)
standard_schemes->push_back(kShellStandardURLSchemes[i]);

savable_schemes->push_back(kExtensionScheme);
standard_schemes->push_back(kExtensionResourceScheme);
savable_schemes->push_back(kExtensionResourceScheme);
}

Expand Down
3 changes: 2 additions & 1 deletion extensions/shell/common/shell_content_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "base/compiler_specific.h"
#include "base/macros.h"
#include "content/public/common/content_client.h"
#include "url/url_util.h"

namespace extensions {

Expand All @@ -18,7 +19,7 @@ class ShellContentClient : public content::ContentClient {

void AddPepperPlugins(
std::vector<content::PepperPluginInfo>* plugins) override;
void AddAdditionalSchemes(std::vector<std::string>* standard_schemes,
void AddAdditionalSchemes(std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* saveable_shemes) override;
std::string GetUserAgent() const override;
base::string16 GetLocalizedString(int message_id) const override;
Expand Down
15 changes: 12 additions & 3 deletions extensions/test/extensions_unittests_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,17 @@
#include "third_party/mojo/src/mojo/edk/embedder/test_embedder.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gl/test/gl_surface_test_support.h"
#include "url/url_util.h"

namespace {

const int kNumExtensionStandardURLSchemes = 2;
const url::SchemeWithType kExtensionStandardURLSchemes[
kNumExtensionStandardURLSchemes] = {
{extensions::kExtensionScheme, url::SCHEME_WITHOUT_PORT},
{extensions::kExtensionResourceScheme, url::SCHEME_WITHOUT_PORT},
};

// Content client that exists only to register chrome-extension:// scheme with
// the url module.
// TODO(jamescook): Should this be merged with ShellContentClient? Should this
Expand All @@ -30,11 +38,12 @@ class ExtensionsContentClient : public content::ContentClient {

// content::ContentClient overrides:
void AddAdditionalSchemes(
std::vector<std::string>* standard_schemes,
std::vector<url::SchemeWithType>* standard_schemes,
std::vector<std::string>* savable_schemes) override {
standard_schemes->push_back(extensions::kExtensionScheme);
for (int i = 0; i < kNumExtensionStandardURLSchemes; i++)
standard_schemes->push_back(kExtensionStandardURLSchemes[i]);

savable_schemes->push_back(extensions::kExtensionScheme);
standard_schemes->push_back(extensions::kExtensionResourceScheme);
savable_schemes->push_back(extensions::kExtensionResourceScheme);
}

Expand Down
2 changes: 1 addition & 1 deletion google_apis/drive/drive_api_url_generator_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ TEST_F(DriveApiUrlGeneratorTest, GetFilesGetUrl) {
url_generator_.GetFilesGetUrl("0ADK06pfg", true, GURL()).spec());

// If |embed_origin| is not empty, it should be added as a query parameter.
url::AddStandardScheme("chrome-extension");
url::AddStandardScheme("chrome-extension", url::SCHEME_WITHOUT_PORT);
EXPECT_EQ(
"https://www.example.com/drive/v2/files/0ADK06pfg"
"?embedOrigin=chrome-extension%3A%2F%2Ftest",
Expand Down
3 changes: 2 additions & 1 deletion ios/chrome/test/ios_chrome_unit_test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ void IOSChromeUnitTestSuite::Initialize() {

{
ios::TestChromeBrowserProvider provider;
url::AddStandardScheme(provider.GetChromeUIScheme());
url::AddStandardScheme(provider.GetChromeUIScheme(),
url::SCHEME_WITHOUT_PORT);
}

base::TestSuite::Initialize();
Expand Down
2 changes: 1 addition & 1 deletion mojo/runner/url_resolver.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace runner {

URLResolver::URLResolver() {
// Needed to treat first component of mojo URLs as host, not path.
url::AddStandardScheme("mojo");
url::AddStandardScheme("mojo", url::SCHEME_WITHOUT_AUTHORITY);
}

URLResolver::~URLResolver() {
Expand Down
Loading

0 comments on commit 11a7c9f

Please sign in to comment.