Skip to content

Commit

Permalink
Remove NetworkServiceRestartBrowserTest.Plugin.
Browse files Browse the repository at this point in the history
This test, along with the CORB test PPAPI plugin, were used to ensure
the PPAPI PDF plugin behaved correctly. Delete them now that the PPAPI
PDF plugin is gone.

Bug: 1302684
Change-Id: Ib9624774a7a4028b6f7c48dc85c2ea585971dd82
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3506231
Reviewed-by: K. Moon <kmoon@chromium.org>
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Derek Schuff <dschuff@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/main@{#979388}
  • Loading branch information
leizleiz authored and Chromium LUCI CQ committed Mar 9, 2022
1 parent b2f48d9 commit 88b95f8
Show file tree
Hide file tree
Showing 12 changed files with 1 addition and 306 deletions.
1 change: 0 additions & 1 deletion components/nacl/browser/nacl_process_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
#include "ppapi/host/host_factory.h"
#include "ppapi/host/ppapi_host.h"
#include "ppapi/proxy/ppapi_messages.h"
#include "ppapi/shared_impl/ppapi_constants.h"
#include "ppapi/shared_impl/ppapi_nacl_plugin_args.h"
#include "sandbox/policy/mojom/sandbox.mojom.h"
#include "sandbox/policy/switches.h"
Expand Down
100 changes: 0 additions & 100 deletions content/browser/network_service_restart_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@
#include "testing/gmock/include/gmock/gmock.h"
#include "third_party/blink/public/common/features.h"

#if BUILDFLAG(ENABLE_PLUGINS)
#include "content/public/test/ppapi_test_utils.h"
#endif

namespace content {

namespace {
Expand Down Expand Up @@ -140,12 +136,6 @@ class NetworkServiceRestartBrowserTest : public ContentBrowserTest {
const NetworkServiceRestartBrowserTest&) = delete;

void SetUpCommandLine(base::CommandLine* command_line) override {
#if BUILDFLAG(ENABLE_PLUGINS)
// TODO(lukasza, kmoon): https://crbug.com/702993: Remove this dependency
// (and //ppapi/tests/corb_test_plugin.cc + BUILD.gn dependencies) once
// PDF support doesn't depend on PPAPI anymore.
ASSERT_TRUE(ppapi::RegisterCorbTestPlugin(command_line));
#endif
ContentBrowserTest::SetUpCommandLine(command_line);
}

Expand Down Expand Up @@ -1012,96 +1002,6 @@ IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, Cookies) {
EXPECT_EQ("foo=bar", EvalJs(web_contents, "document.cookie;"));
}

#if BUILDFLAG(ENABLE_PLUGINS)
// Make sure that "trusted" plugins continue to be able to issue requests that
// are cross-origin (wrt the host frame) after a network process crash:
// - html frame: main-frame.com/title1.html
// \-- plugin document: cross.origin.com/.../js.txt (`plugin_document_url`)
// \-- request from plugin: cross.origin.com/.../js.txt
// This mimics the behavior of PDFs, which only issue requests for the plugin
// document (e.q. network::ResourceRequest::request_initiator is same-origin wrt
// ResourceRequest::url).
//
// This primarily verifies that OnNetworkServiceCrashRestorePluginExceptions in
// render_process_host_impl.cc refreshes AddAllowedRequestInitiatorForPlugin
// data after a NetworkService crash.
//
// See also https://crbug.com/874515 and https://crbug.com/846339.
//
// TODO(lukasza, kmoon): https://crbug.com/702993: Remove this test once PDF
// support doesn't depend on PPAPI anymore.
IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest, Plugin) {
if (IsInProcessNetworkService())
return;
auto* web_contents = shell()->web_contents();
ASSERT_TRUE(NavigateToURL(
web_contents,
embedded_test_server()->GetURL("main.frame.com", "/title1.html")));

// Load cross-origin document into the test plugin (see
// ppapi::RegisterCorbTestPlugin).
//
// The document has to be a MIME type that CORB will allow (such as
// javascript) - it cannot be a pdf or json, because these would be blocked by
// CORB (the real PDF plugin works because the plugin is hosted in a Chrome
// Extension where CORB is turned off).
GURL plugin_document_url = embedded_test_server()->GetURL(
"cross.origin.com", "/site_isolation/js.txt");
const char kLoadingScriptTemplate[] = R"(
var obj = document.createElement('object');
obj.id = 'plugin';
obj.data = $1;
obj.type = 'application/x-fake-pdf-for-testing';
obj.width = 400;
obj.height = 400;
document.body.appendChild(obj);
)";
EXPECT_FALSE(
web_contents->GetMainFrame()->GetLastCommittedOrigin().IsSameOriginWith(
url::Origin::Create(plugin_document_url)));
ASSERT_TRUE(ExecJs(web_contents,
JsReplace(kLoadingScriptTemplate, plugin_document_url)));

// Ask the plugin to re-request the document URL (similarly to what the PDF
// plugin does to get chunks of linearized PDFs).
const char kFetchScriptTemplate[] = R"(
new Promise(function (resolve, reject) {
var obj = document.getElementById('plugin');
function callback(event) {
// Ignore plugin messages unrelated to requestUrl.
if (!event.data.startsWith('requestUrl: '))
return;
obj.removeEventListener('message', callback);
resolve('msg-from-plugin: ' + event.data);
};
obj.addEventListener('message', callback);
obj.postMessage('requestUrl: ' + $1);
});
)";
std::string fetch_script =
JsReplace(kFetchScriptTemplate, plugin_document_url);
ASSERT_EQ(
"msg-from-plugin: requestUrl: RESPONSE BODY: "
"var j = 0; document.write(j);\n",
EvalJs(web_contents, fetch_script));

// Crash the Network Service process and wait until host frame's
// URLLoaderFactory has been refreshed.
SimulateNetworkServiceCrash();
main_frame()->FlushNetworkAndNavigationInterfacesForTesting();

// Try the fetch again - it should still work (i.e. the mechanism for relaxing
// request_initiator_origin_lock enforcement should be resilient to network
// process crashes).
ASSERT_EQ(
"msg-from-plugin: requestUrl: RESPONSE BODY: "
"var j = 0; document.write(j);\n",
EvalJs(web_contents, fetch_script));
}
#endif

// TODO(crbug.com/901026): Fix deadlock on process startup on Android.
#if BUILDFLAG(IS_ANDROID)
IN_PROC_BROWSER_TEST_F(NetworkServiceRestartBrowserTest,
Expand Down
33 changes: 0 additions & 33 deletions content/browser/plugin_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,39 +149,6 @@ PpapiPluginProcessHost* PluginServiceImpl::FindOrStartPpapiPluginProcess(
return nullptr;
}

if (info->permissions & ppapi::PERMISSION_PDF) {
// Extra assertions for the PDF plugin. These assertions do not apply to
// the test plugin.
if (0 == (info->permissions & ppapi::PERMISSION_TESTING)) {
// We want to limit ability to bypass |request_initiator_origin_lock| to
// trustworthy renderers. PDF plugin is okay, because it is always hosted
// by the PDF extension (mhjfbmdgcfjbbpaeojofohoefgiehjai) or
// chrome://print, both of which we assume are trustworthy (the extension
// process can also host other extensions, but this is okay).
//
// The CHECKs below help verify that |render_process_id| does not host
// web-controlled content. This is a defense-in-depth for verifying that
// ShouldAllowPluginCreation called above is doing the right thing.
auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
ProcessLock renderer_lock = policy->GetProcessLock(render_process_id);
CHECK(!renderer_lock.matches_scheme(url::kHttpScheme) &&
!renderer_lock.matches_scheme(url::kHttpsScheme));
CHECK(embedder_origin.scheme() != url::kHttpScheme);
CHECK(embedder_origin.scheme() != url::kHttpsScheme);
CHECK(!embedder_origin.opaque());
}

// In some scenarios, the PDF plugin can issue fetch requests that will need
// to be proxied by |render_process_id| - such proxying needs to bypass
// CORB. See also https://crbug.com/1027173.
//
// TODO(lukasza, kmoon): https://crbug.com/702993: Remove the code here once
// PDF support doesn't depend on PPAPI anymore.
DCHECK(origin_lock.has_value());
RenderProcessHostImpl::AddAllowedRequestInitiatorForPlugin(
render_process_id, origin_lock.value());
}

PpapiPluginProcessHost* plugin_host =
FindPpapiPluginProcess(plugin_path, profile_data_directory, origin_lock);
if (plugin_host)
Expand Down
21 changes: 0 additions & 21 deletions content/public/test/ppapi_test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "content/browser/renderer_host/pepper/pepper_udp_socket_message_filter.h"
#include "content/public/common/content_constants.h"
#include "content/public/common/content_switches.h"
#include "ppapi/shared_impl/ppapi_constants.h"
#include "ppapi/shared_impl/ppapi_switches.h"

using CharType = base::FilePath::CharType;
Expand Down Expand Up @@ -72,20 +71,6 @@ bool RegisterPluginWithDefaultMimeType(
return RegisterPlugins(command_line, plugins);
}

bool RegisterFakePdfPluginLibrary(base::CommandLine* command_line,
const StringType& library_name) {
std::vector<PluginInfo> plugins;
// Register a fake PDF plugin with 100.0 version (to avoid outdated checks).
base::FilePath::StringType fake_pdf_parameter =
base::FilePath::FromUTF8Unsafe(std::string("#") + "Fake PDF" +
"#Description#100.0")
.value();
plugins.push_back(
PluginInfo(library_name, fake_pdf_parameter,
FILE_PATH_LITERAL("application/x-fake-pdf-for-testing")));
return RegisterPlugins(command_line, plugins);
}

} // namespace

bool RegisterTestPlugin(base::CommandLine* command_line) {
Expand All @@ -107,12 +92,6 @@ bool RegisterTestPluginWithExtraParameters(
extra_registration_parameters);
}

bool RegisterCorbTestPlugin(base::CommandLine* command_line) {
StringType library_name =
base::FilePath::FromUTF8Unsafe(ppapi::kCorbTestPluginName).value();
return RegisterFakePdfPluginLibrary(command_line, library_name);
}

bool RegisterBlinkTestPlugin(base::CommandLine* command_line) {
#if BUILDFLAG(IS_WIN)
static const CharType kPluginLibrary[] = L"blink_test_plugin.dll";
Expand Down
3 changes: 0 additions & 3 deletions content/public/test/ppapi_test_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,6 @@ namespace ppapi {
base::CommandLine* command_line,
const base::FilePath::StringType& extra_registration_parameters);

// Registers the PDF-imitating CORB-testing plugin.
[[nodiscard]] bool RegisterCorbTestPlugin(base::CommandLine* command_line);

// Registers the Blink test plugin to application/x-blink-test-plugin.
[[nodiscard]] bool RegisterBlinkTestPlugin(base::CommandLine* command_line);

Expand Down
5 changes: 1 addition & 4 deletions content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1540,10 +1540,7 @@ test("content_browsertests") {
"//ppapi/proxy:ipc",
"//ppapi/shared_impl:test_support",
]
data_deps += [
"//ppapi:corb_test_plugin",
"//ppapi:ppapi_tests",
]
data_deps += [ "//ppapi:ppapi_tests" ]
if (is_mac) {
data += [ "$root_out_dir/ppapi_tests.plugin/" ]
}
Expand Down
25 changes: 0 additions & 25 deletions ppapi/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -219,31 +219,6 @@ target(ppapi_tests_target_type, "ppapi_tests") {
deps = [ ":ppapi_tests_sources" ]
}

# Test plugin for Cross-Origin Read Blocking.
# See also ppapi::RegisterCorbTestPlugin.
source_set("corb_test_plugin_sources") {
sources = [
"tests/corb_test_plugin.cc",
"tests/test_utils.cc",
"tests/test_utils.h",
]

deps = [
"//ppapi/cpp",
"//ppapi/shared_impl",
]
}

if (!is_mac) {
shared_library("corb_test_plugin") {
deps = [ ":corb_test_plugin_sources" ]
}
} else {
mac_plugin_bundle("corb_test_plugin") {
deps = [ ":corb_test_plugin_sources" ]
}
}

source_set("blink_deprecated_test_plugin_sources") {
sources = [ "tests/blink_deprecated_test_plugin.cc" ]

Expand Down
1 change: 0 additions & 1 deletion ppapi/proxy/plugin_globals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include "ppapi/proxy/ppb_message_loop_proxy.h"
#include "ppapi/proxy/resource_reply_thread_registrar.h"
#include "ppapi/proxy/udp_socket_filter.h"
#include "ppapi/shared_impl/ppapi_constants.h"
#include "ppapi/shared_impl/proxy_lock.h"
#include "ppapi/thunk/enter.h"

Expand Down
1 change: 0 additions & 1 deletion ppapi/shared_impl/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,6 @@ component("shared_impl") {
"media_stream_video_track_shared.h",
"platform_file.cc",
"platform_file.h",
"ppapi_constants.h",
"ppapi_nacl_plugin_args.cc",
"ppapi_nacl_plugin_args.h",
"ppapi_permissions.cc",
Expand Down
23 changes: 0 additions & 23 deletions ppapi/shared_impl/ppapi_constants.h

This file was deleted.

1 change: 0 additions & 1 deletion ppapi/shared_impl/ppapi_nacl_plugin_args.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "ppapi/shared_impl/ppapi_constants.h"
#include "ppapi/shared_impl/ppapi_nacl_plugin_args.h"

namespace ppapi {
Expand Down
Loading

0 comments on commit 88b95f8

Please sign in to comment.