Commit ff34ad77 authored by Tom Sepez's avatar Tom Sepez Committed by Commit Bot

Re-land "Don't share PDF plugins across origin boundaries."

This reverts commit 5ff974fc.

Original CL was https://chromium-review.googlesource.com/953979
This CL differs from the original in that in the test framework,
PDFs are loaded into separate tabs, instead of on top of each
other. Under slow test infrastructure (e.g. MSAN), the plugin
processes would exit before we could complete the loads and
count them. This fixes the test (and the shipping code is unchanged
from the original).

TBR=jam@chromium.org

Bug: 809614
Change-Id: Id13a97023a60d86fb72c47d580c001b737bf17bf
Reviewed-on: https://chromium-review.googlesource.com/959236
Commit-Queue: Tom Sepez <tsepez@chromium.org>
Reviewed-by: default avatarTom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542652}
parent 70e94f29
......@@ -55,6 +55,7 @@
#include "content/public/browser/render_widget_host.h"
#include "content/public/browser/render_widget_host_view.h"
#include "content/public/browser/web_contents.h"
#include "content/public/common/content_features.h"
#include "content/public/common/context_menu_params.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/test_navigation_observer.h"
......@@ -198,6 +199,15 @@ class PDFExtensionTest : public ExtensionApiTest,
return pdf_extension_test_util::EnsurePDFHasLoaded(web_contents);
}
// Same as LoadPDF(), but loads into a new tab.
bool LoadPdfInNewTab(const GURL& url) {
ui_test_utils::NavigateToURLWithDisposition(
browser(), url, WindowOpenDisposition::NEW_FOREGROUND_TAB,
ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
WebContents* web_contents = GetActiveWebContents();
return pdf_extension_test_util::EnsurePDFHasLoaded(web_contents);
}
// Same as LoadPdf(), but also returns a pointer to the guest WebContents for
// the loaded PDF. Returns nullptr if the load fails.
WebContents* LoadPdfGetGuestContents(const GURL& url) {
......@@ -887,6 +897,42 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionTest, OpenFromFTP) {
EXPECT_EQ(base::ASCIIToUTF16("test.pdf"), GetActiveWebContents()->GetTitle());
}
IN_PROC_BROWSER_TEST_F(PDFExtensionTest, MultipleDomains) {
for (const auto& url :
{embedded_test_server()->GetURL("a.com", "/pdf/test.pdf"),
embedded_test_server()->GetURL("b.com", "/pdf/test.pdf"),
embedded_test_server()->GetURL("c.com", "/pdf/test.pdf"),
embedded_test_server()->GetURL("d.com", "/pdf/test.pdf")}) {
ASSERT_TRUE(LoadPdfInNewTab(url));
}
EXPECT_EQ(1, CountPDFProcesses());
}
class PDFIsolatedExtensionTest : public PDFExtensionTest {
public:
PDFIsolatedExtensionTest() {}
~PDFIsolatedExtensionTest() override {}
void SetUp() override {
features_.InitAndEnableFeature(features::kPdfIsolation);
PDFExtensionTest::SetUp();
}
private:
base::test::ScopedFeatureList features_;
};
IN_PROC_BROWSER_TEST_F(PDFIsolatedExtensionTest, MultipleDomains) {
for (const auto& url :
{embedded_test_server()->GetURL("a.com", "/pdf/test.pdf"),
embedded_test_server()->GetURL("b.com", "/pdf/test.pdf"),
embedded_test_server()->GetURL("c.com", "/pdf/test.pdf"),
embedded_test_server()->GetURL("d.com", "/pdf/test.pdf")}) {
ASSERT_TRUE(LoadPdfInNewTab(url));
}
EXPECT_EQ(4, CountPDFProcesses());
}
class PDFExtensionLinkClickTest : public PDFExtensionTest {
public:
PDFExtensionLinkClickTest() : guest_contents_(nullptr) {}
......
......@@ -1372,6 +1372,12 @@ bool ChromeContentRendererClient::IsExternalPepperPlugin(
return module_name == "Native Client";
}
bool ChromeContentRendererClient::IsOriginIsolatedPepperPlugin(
const base::FilePath& plugin_path) {
return plugin_path ==
base::FilePath::FromUTF8Unsafe(ChromeContentClient::kPDFPluginPath);
}
#if BUILDFLAG(ENABLE_PLUGINS) && BUILDFLAG(ENABLE_EXTENSIONS)
bool ChromeContentRendererClient::IsExtensionOrSharedModuleWhitelisted(
const GURL& url,
......
......@@ -169,6 +169,7 @@ class ChromeContentRendererClient
const content::RenderFrame* render_frame,
blink::mojom::PageVisibilityState* override_state) override;
bool IsExternalPepperPlugin(const std::string& module_name) override;
bool IsOriginIsolatedPepperPlugin(const base::FilePath& plugin_path) override;
std::unique_ptr<blink::WebSocketHandshakeThrottle>
CreateWebSocketHandshakeThrottle() override;
std::unique_ptr<blink::WebSpeechSynthesizer> OverrideSpeechSynthesizer(
......
......@@ -244,6 +244,10 @@ const base::Feature kPassiveDocumentEventListeners{
const base::Feature kPassiveEventListenersDueToFling{
"PassiveEventListenersDueToFling", base::FEATURE_ENABLED_BY_DEFAULT};
// Whether PDF files should be rendered in diffent processes based on origin.
const base::Feature kPdfIsolation = {"PdfIsolation",
base::FEATURE_DISABLED_BY_DEFAULT};
// If Pepper 3D Image Chromium is allowed, this feature controls whether it is
// enabled.
const base::Feature kPepper3DImageChromium {
......
......@@ -64,6 +64,7 @@ CONTENT_EXPORT extern const base::Feature kOriginManifest;
CONTENT_EXPORT extern const base::Feature kOriginTrials;
CONTENT_EXPORT extern const base::Feature kPassiveDocumentEventListeners;
CONTENT_EXPORT extern const base::Feature kPassiveEventListenersDueToFling;
CONTENT_EXPORT extern const base::Feature kPdfIsolation;
CONTENT_EXPORT extern const base::Feature kPepper3DImageChromium;
CONTENT_EXPORT extern const base::Feature kPurgeAndSuspend;
CONTENT_EXPORT extern const base::Feature kPWAFullCodeCache;
......
......@@ -173,6 +173,11 @@ bool ContentRendererClient::IsExternalPepperPlugin(
return false;
}
bool ContentRendererClient::IsOriginIsolatedPepperPlugin(
const base::FilePath& plugin_path) {
return false;
}
bool ContentRendererClient::AllowPepperMediaStreamAPI(const GURL& url) {
return false;
}
......
......@@ -13,6 +13,7 @@
#include <vector>
#include "base/callback_forward.h"
#include "base/files/file_path.h"
#include "base/memory/ref_counted.h"
#include "base/strings/string16.h"
#include "base/task_scheduler/task_scheduler.h"
......@@ -261,6 +262,12 @@ class CONTENT_EXPORT ContentRendererClient {
// startup steps).
virtual bool IsExternalPepperPlugin(const std::string& module_name);
// Returns true if the given Pepper plugin should process content from
// different origins in different PPAPI processes. This is generally a
// worthwhile precaution when the plugin provides an active scripting
// language.
virtual bool IsOriginIsolatedPepperPlugin(const base::FilePath& plugin_path);
// Returns true if the page at |url| can use Pepper MediaStream APIs.
virtual bool AllowPepperMediaStreamAPI(const GURL& url);
......
......@@ -2852,8 +2852,11 @@ blink::WebPlugin* RenderFrameImpl::CreatePlugin(
this, delegate->GetWeakPtr());
}
// TODO(tsepez): extract origin to lock from WebPluginParams url.
base::Optional<url::Origin> origin_lock;
if (base::FeatureList::IsEnabled(features::kPdfIsolation) &&
GetContentClient()->renderer()->IsOriginIsolatedPepperPlugin(info.path)) {
origin_lock = url::Origin::Create(GURL(params.url));
}
bool pepper_plugin_was_registered = false;
scoped_refptr<PluginModule> pepper_module(PluginModule::Create(
......
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