Skip to content

Commit

Permalink
Revert "PPAPI: Add NetworkIsolationKeys to proxy lookup requests."
Browse files Browse the repository at this point in the history
This reverts commit 9939a33.

Reason for revert: PPAPINaClNewlibTest.NetworkProxy test failing on Mac ASan 64 Tests (1)
Fixed: 1044845

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.
> 
> Bug: 1021661
> Change-Id: Ibbca8db7df8c7e61cb9522630e70c8ccd365b5c7
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2008050
> Commit-Queue: Matt Menke <mmenke@chromium.org>
> Reviewed-by: Raymes Khoury <raymes@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#734094}

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

Change-Id: Ia2c799af73dff4749b1af077b7d976f192d785aa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1021661
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2015872
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734343}
  • Loading branch information
Yoichi Osato authored and Commit Bot committed Jan 23, 2020
1 parent 8b44721 commit 51fc3ec
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 53 deletions.
37 changes: 4 additions & 33 deletions chrome/test/ppapi/ppapi_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1277,9 +1277,7 @@ TEST_PPAPI_NACL_DISALLOWED_SOCKETS(UDPSocketPrivateDisallowed)
// 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
// looked up across tabs with different first party origins.
void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
Browser* browser,
bool include_canonical_name) {
void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(Browser* browser) {
network::mojom::NetworkContext* network_context =
content::BrowserContext::GetDefaultStoragePartition(browser->profile())
->GetNetworkContext();
Expand All @@ -1291,7 +1289,7 @@ void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
// Cache only lookup.
params->source = net::HostResolverSource::LOCAL_ONLY;
// Match the parameters used by the test.
params->include_canonical_name = include_canonical_name;
params->include_canonical_name = true;
net::NetworkIsolationKey network_isolation_key =
browser->tab_strip_model()
->GetActiveWebContents()
Expand Down Expand Up @@ -1323,8 +1321,7 @@ void CheckTestHostNameUsedWithCorrectNetworkIsolationKey(
#define RUN_HOST_RESOLVER_SUBTESTS \
RunTestViaHTTP(LIST_TEST(HostResolver_Empty) LIST_TEST(HostResolver_Resolve) \
LIST_TEST(HostResolver_ResolveIPv4)); \
CheckTestHostNameUsedWithCorrectNetworkIsolationKey( \
browser(), true /* include_canonical_name */)
CheckTestHostNameUsedWithCorrectNetworkIsolationKey(browser())

IN_PROC_BROWSER_TEST_F(OutOfProcessPPAPITest, HostResolverCrash_Basic) {
if (content::IsInProcessNetworkService())
Expand Down Expand Up @@ -2169,33 +2166,7 @@ TEST_PPAPI_NACL(MAYBE_MediaStreamVideoTrack)

TEST_PPAPI_NACL(MouseCursor)

// Proxy configuration test. The PPAPI code used by these tests is in
// 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 */);
}
TEST_PPAPI_NACL(NetworkProxy)

// TODO(crbug.com/619765): get working on CrOS build.
#if defined(OS_CHROMEOS)
Expand Down
15 changes: 0 additions & 15 deletions chrome/test/ppapi/ppapi_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,24 +160,9 @@ void PPAPITestBase::SetUpCommandLine(base::CommandLine* command_line) {

// Smooth scrolling confuses the scrollbar test.
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') {"
" dnsResolveEx('host_resolver.test');"
" return 'PROXY proxy.test';"
" }"
" return 'DIRECT'"
"}");
}

void PPAPITestBase::SetUpOnMainThread() {
host_resolver()->AddRule("host_resolver.test",
embedded_test_server()->host_port_pair().host());
host_resolver()->AddRuleWithFlags(
"host_resolver.test", embedded_test_server()->host_port_pair().host(),
net::HOST_RESOLVER_CANONNAME);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ bool LookUpProxyForURLCallback(
StoragePartition* storage_partition = BrowserContext::GetStoragePartition(
site_instance->GetBrowserContext(), site_instance);

// TODO(https://crbug.com/1021661): Pass in a non-empty NetworkIsolationKey.
storage_partition->GetNetworkContext()->LookUpProxyForURL(
url, render_frame_host->GetNetworkIsolationKey(),
std::move(proxy_lookup_client));
url, net::NetworkIsolationKey::Todo(), std::move(proxy_lookup_client));
return true;
}

Expand Down
2 changes: 2 additions & 0 deletions content/test/ppapi/ppapi_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ TEST_PPAPI_OUT_OF_PROCESS(MessageHandler)
TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Basics)
TEST_PPAPI_OUT_OF_PROCESS(MessageLoop_Post)

TEST_PPAPI_OUT_OF_PROCESS(NetworkProxy)

// TODO(danakj): http://crbug.com/115286
TEST_PPAPI_IN_PROCESS(DISABLED_Scrollbar)
// http://crbug.com/89961
Expand Down
9 changes: 6 additions & 3 deletions ppapi/tests/test_network_proxy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,15 @@ std::string TestNetworkProxy::TestGetProxyForURL() {
// Assume no one configures a proxy for localhost.
ASSERT_EQ("DIRECT", callback.output().AsString());

callback.WaitForResult(pp::NetworkProxy::GetProxyForURL(
instance_, pp::Var("https://use.proxy.test/"), callback.GetCallback()));
callback.WaitForResult(
pp::NetworkProxy::GetProxyForURL(instance_,
pp::Var("http://www.google.com"),
callback.GetCallback()));
CHECK_CALLBACK_BEHAVIOR(callback);
ASSERT_EQ(PP_OK, callback.result());
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(
pp::NetworkProxy::GetProxyForURL(instance_,
Expand Down

0 comments on commit 51fc3ec

Please sign in to comment.