Commit aaf52161 authored by Hajime Hoshi's avatar Hajime Hoshi Committed by Commit Bot

Revert "PPAPI: Add NetworkIsolationKeys to proxy lookup requests."

This reverts commit 45fb6502.

Reason for revert: Test failure: crbug.com/1051400

Original change's description:
> PPAPI:  Add NetworkIsolationKeys to proxy lookup requests.
> 
> This is needed to correctly isolate any DNS requests made by PPAPI in
> the context of one page from those made in the context of other pages.
> 
> This is a reland of
> https://chromium-review.googlesource.com/c/chromium/src/+/2008050. The
> original CL was reverted because the DNS layer auto-detects whether the
> current network supports IPv6, and the detection value was changing in
> the middle of tests. When the value changes, the cache key used for the
> DNS layer changes, which was resulting in cache misses.
> 
> To fix the issue, the test now relies on IPv4 DNS lookups, which don't
> have this issue.
> 
> Bug: 1021661
> Change-Id: I959b81d0a3fc5973cde4ba7a0ccfd0c501ffc235
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050743
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Raymes Khoury <raymes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#740487}

TBR=raymes@chromium.org,mmenke@chromium.org

Change-Id: I40432831baa19cdf040c38684fb785f48816873f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1021661
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2051644Reviewed-by: default avatarHajime Hoshi <hajimehoshi@chromium.org>
Commit-Queue: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740580}
parent f922b584
...@@ -1269,17 +1269,7 @@ TEST_PPAPI_NACL_DISALLOWED_SOCKETS(UDPSocketPrivateDisallowed) ...@@ -1269,17 +1269,7 @@ TEST_PPAPI_NACL_DISALLOWED_SOCKETS(UDPSocketPrivateDisallowed)
// is present in the DNS cache with the NetworkIsolationKey associated with the // is present in the DNS cache with the NetworkIsolationKey associated with the
// foreground WebContents - this is needed so as not to leak what hostnames were // foreground WebContents - this is needed so as not to leak what hostnames were
// looked up across tabs with different first party origins. // looked up across tabs with different first party origins.
// void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(Browser* browser) {
// The lookup checks for an IPv4-only cache entry, as UNSPECIFIED lookups are
// sometimes replaced with IPv4-only lookups, based on the result of that probe
// of whether the local network has IPv6 connectivity. For unknown reasons, the
// results of this probe can change in the middle of a test on the bots, which
// changes the used cache key. To avoid that issue, just use A lookups.
//
// See https://crbug.com/1042354 for some discussion of the issue.
void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
Browser* browser,
bool include_canonical_name) {
network::mojom::NetworkContext* network_context = network::mojom::NetworkContext* network_context =
content::BrowserContext::GetDefaultStoragePartition(browser->profile()) content::BrowserContext::GetDefaultStoragePartition(browser->profile())
->GetNetworkContext(); ->GetNetworkContext();
...@@ -1290,10 +1280,8 @@ void CheckTestHostNameUsedWithCorrectNetworkIsolationKey( ...@@ -1290,10 +1280,8 @@ void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
network::mojom::ResolveHostParameters::New(); network::mojom::ResolveHostParameters::New();
// Cache only lookup. // Cache only lookup.
params->source = net::HostResolverSource::LOCAL_ONLY; params->source = net::HostResolverSource::LOCAL_ONLY;
// A-only lookup.
params->dns_query_type = net::DnsQueryType::A;
// Match the parameters used by the test. // Match the parameters used by the test.
params->include_canonical_name = include_canonical_name; params->include_canonical_name = true;
net::NetworkIsolationKey network_isolation_key = net::NetworkIsolationKey network_isolation_key =
browser->tab_strip_model() browser->tab_strip_model()
->GetActiveWebContents() ->GetActiveWebContents()
...@@ -1325,8 +1313,7 @@ void CheckTestHostNameUsedWithCorrectNetworkIsolationKey( ...@@ -1325,8 +1313,7 @@ void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
#define RUN_HOST_RESOLVER_SUBTESTS \ #define RUN_HOST_RESOLVER_SUBTESTS \
RunTestViaHTTP(LIST_TEST(HostResolver_Empty) LIST_TEST(HostResolver_Resolve) \ RunTestViaHTTP(LIST_TEST(HostResolver_Empty) LIST_TEST(HostResolver_Resolve) \
LIST_TEST(HostResolver_ResolveIPv4)); \ LIST_TEST(HostResolver_ResolveIPv4)); \
CheckTestHostNameUsedWithCorrectNetworkIsolationKey( \ CheckTestHostNameUsedWithCorrectNetworkIsolationKey(browser())
browser(), true /* include_canonical_name */)
IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, HostResolverCrash_Basic) { IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, HostResolverCrash_Basic) {
if (content::IsInProcessNetworkService()) if (content::IsInProcessNetworkService())
...@@ -2195,33 +2182,7 @@ TEST_PPAPI_NACL(MAYBE_MediaStreamVideoTrack) ...@@ -2195,33 +2182,7 @@ TEST_PPAPI_NACL(MAYBE_MediaStreamVideoTrack)
TEST_PPAPI_NACL(MouseCursor) TEST_PPAPI_NACL(MouseCursor)
// Proxy configuration test. The PPAPI code used by these tests is in TEST_PPAPI_NACL(NetworkProxy)
// ppapi/tests/test_network_proxy.cc.
IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, NetworkProxy) {
RunTestViaHTTP(STRIP_PREFIXES(NetworkProxy));
CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
browser(), false /* include_canonical_name */);
}
IN_PROC_BROWSER_TEST_F(PPAPINaClNewlibTest, NetworkProxy) {
RunTestViaHTTP(STRIP_PREFIXES(NetworkProxy));
CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
browser(), false /* include_canonical_name */);
}
IN_PROC_BROWSER_TEST_F(PPAPINaClPNaClTest, MAYBE_PPAPI_NACL(NetworkProxy)) {
RunTestViaHTTP(STRIP_PREFIXES(NetworkProxy));
CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
browser(), false /* include_canonical_name */);
}
IN_PROC_BROWSER_TEST_F(PPAPINaClPNaClNonSfiTest,
MAYBE_PNACL_NONSFI(NetworkProxy)) {
RunTestViaHTTP(STRIP_PREFIXES(NetworkProxy));
CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
browser(), false /* include_canonical_name */);
}
// TODO(crbug.com/619765): get working on CrOS build. // TODO(crbug.com/619765): get working on CrOS build.
#if defined(OS_CHROMEOS) #if defined(OS_CHROMEOS)
......
...@@ -160,24 +160,9 @@ void PPAPITestBase::SetUpCommandLine(base::CommandLine* command_line) { ...@@ -160,24 +160,9 @@ void PPAPITestBase::SetUpCommandLine(base::CommandLine* command_line) {
// Smooth scrolling confuses the scrollbar test. // Smooth scrolling confuses the scrollbar test.
command_line->AppendSwitch(switches::kDisableSmoothScrolling); command_line->AppendSwitch(switches::kDisableSmoothScrolling);
// For a particular host name, resolve another specific host name (which
// should then be added to the DNS cache), and then return a particular proxy.
// Otherwise, return DIRECT.
command_line->AppendSwitchASCII(switches::kProxyPacUrl,
"data:,"
"function FindProxyForURL(url, host) {"
" if (host == 'use.proxy.test') {"
" dnsResolve('host_resolver.test');"
" return 'PROXY proxy.test';"
" }"
" return 'DIRECT'"
"}");
} }
void PPAPITestBase::SetUpOnMainThread() { void PPAPITestBase::SetUpOnMainThread() {
host_resolver()->AddRule("host_resolver.test",
embedded_test_server()->host_port_pair().host());
host_resolver()->AddRuleWithFlags( host_resolver()->AddRuleWithFlags(
"host_resolver.test", embedded_test_server()->host_port_pair().host(), "host_resolver.test", embedded_test_server()->host_port_pair().host(),
net::HOST_RESOLVER_CANONNAME); net::HOST_RESOLVER_CANONNAME);
......
...@@ -46,9 +46,9 @@ bool LookUpProxyForURLCallback( ...@@ -46,9 +46,9 @@ bool LookUpProxyForURLCallback(
StoragePartition* storage_partition = BrowserContext::GetStoragePartition( StoragePartition* storage_partition = BrowserContext::GetStoragePartition(
site_instance->GetBrowserContext(), site_instance); site_instance->GetBrowserContext(), site_instance);
// TODO(https://crbug.com/1021661): Pass in a non-empty NetworkIsolationKey.
storage_partition->GetNetworkContext()->LookUpProxyForURL( storage_partition->GetNetworkContext()->LookUpProxyForURL(
url, render_frame_host->GetNetworkIsolationKey(), url, net::NetworkIsolationKey::Todo(), std::move(proxy_lookup_client));
std::move(proxy_lookup_client));
return true; return true;
} }
......
...@@ -122,6 +122,8 @@ TEST_PPAPI_OUT_OF_PROCESS(MessageHandler) ...@@ -122,6 +122,8 @@ TEST_PPAPI_OUT_OF_PROCESS(MessageHandler)
TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Basics) TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Basics)
TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Post) TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Post)
TEST_PPAPI_OUT_OF_PROCESS(NetworkProxy)
// TODO(danakj): http://crbug.com/115286 // TODO(danakj): http://crbug.com/115286
TEST_PPAPI_IN_PROCESS(DISABLED_Scrollbar) TEST_PPAPI_IN_PROCESS(DISABLED_Scrollbar)
// http://crbug.com/89961 // http://crbug.com/89961
......
...@@ -33,12 +33,15 @@ std::string TestNetworkProxy::TestGetProxyForURL() { ...@@ -33,12 +33,15 @@ std::string TestNetworkProxy::TestGetProxyForURL() {
// Assume no one configures a proxy for localhost. // Assume no one configures a proxy for localhost.
ASSERT_EQ("DIRECT", callback.output().AsString()); ASSERT_EQ("DIRECT", callback.output().AsString());
callback.WaitForResult(pp::NetworkProxy::GetProxyForURL( callback.WaitForResult(
instance_, pp::Var("https://use.proxy.test/"), callback.GetCallback())); pp::NetworkProxy::GetProxyForURL(instance_,
pp::Var("http://www.google.com"),
callback.GetCallback()));
CHECK_CALLBACK_BEHAVIOR(callback); CHECK_CALLBACK_BEHAVIOR(callback);
ASSERT_EQ(PP_OK, callback.result()); ASSERT_EQ(PP_OK, callback.result());
output = callback.output(); output = callback.output();
ASSERT_EQ("PROXY proxy.test:80", callback.output().AsString()); // Don't know what the proxy might be, but it should be a valid result.
ASSERT_TRUE(output.is_string());
callback.WaitForResult( callback.WaitForResult(
pp::NetworkProxy::GetProxyForURL(instance_, pp::NetworkProxy::GetProxyForURL(instance_,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment