[unseasoned-pdf] Skip using stream as subresource
The unseasoned PDF viewer (which uses an in-process plugin) will navigate to the stream URL in order to create a browsing context in the correct origin (assisted by PdfNavigationThrottle and PdfURLLoaderRequestInterceptor). MimeHandlerViewGuest registers the stream loader as a subresource, which interferes with this by handing the stream off to the extension page's renderer. As a special case, we'll prevent this when navigating to a PDF extension page. This change also adds a chrome_pdf::features::kPdfUnseasoned feature to control this behavior. Bug: 1123621, 1141953 Change-Id: Iddab8bd66af66f0d8470f1c36ae19967d9c83921 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2937289 Commit-Queue: K. Moon <kmoon@chromium.org> Reviewed-by: Daniel Hosseinian <dhoss@chromium.org> Reviewed-by: Reilly Grant <reillyg@chromium.org> Reviewed-by: W. James MacLean <wjmaclean@chromium.org> Cr-Commit-Position: refs/heads/master@{#889515}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
b210d8ddf8
commit
dd1ed74459
chrome/browser/pdf
extensions/browser
pdf
@ -92,6 +92,7 @@
|
||||
#include "content/public/test/url_loader_interceptor.h"
|
||||
#include "extensions/browser/api/extensions_api_client.h"
|
||||
#include "extensions/browser/extension_registry.h"
|
||||
#include "extensions/browser/guest_view/mime_handler_view/mime_handler_view_guest.h"
|
||||
#include "extensions/common/manifest_handlers/mime_types_handler.h"
|
||||
#include "extensions/test/result_catcher.h"
|
||||
#include "net/dns/mock_host_resolver.h"
|
||||
@ -401,13 +402,9 @@ class PDFExtensionTest : public extensions::ExtensionApiTest {
|
||||
|
||||
protected:
|
||||
// Hooks to set up feature flags.
|
||||
virtual const std::vector<base::Feature> GetEnabledFeatures() const {
|
||||
return {};
|
||||
}
|
||||
virtual std::vector<base::Feature> GetEnabledFeatures() const { return {}; }
|
||||
|
||||
virtual const std::vector<base::Feature> GetDisabledFeatures() const {
|
||||
return {};
|
||||
}
|
||||
virtual std::vector<base::Feature> GetDisabledFeatures() const { return {}; }
|
||||
|
||||
private:
|
||||
WebContents* LoadPdfGetGuestContentsHelper(const GURL& url, bool new_tab) {
|
||||
@ -2937,7 +2934,7 @@ class PDFExtensionAccessibilityTextExtractionTest : public PDFExtensionTest {
|
||||
}
|
||||
|
||||
protected:
|
||||
const std::vector<base::Feature> GetEnabledFeatures() const override {
|
||||
std::vector<base::Feature> GetEnabledFeatures() const override {
|
||||
std::vector<base::Feature> enabled = PDFExtensionTest::GetEnabledFeatures();
|
||||
enabled.push_back(chrome_pdf::features::kAccessiblePDFForm);
|
||||
return enabled;
|
||||
@ -3144,7 +3141,7 @@ class PDFExtensionAccessibilityTreeDumpTest
|
||||
}
|
||||
|
||||
protected:
|
||||
const std::vector<base::Feature> GetEnabledFeatures() const override {
|
||||
std::vector<base::Feature> GetEnabledFeatures() const override {
|
||||
std::vector<base::Feature> enabled = {
|
||||
chrome_pdf::features::kAccessiblePDFForm,
|
||||
};
|
||||
@ -3444,3 +3441,85 @@ IN_PROC_BROWSER_TEST_F(PDFExtensionPrerenderTest,
|
||||
prerender_helper().NavigatePrimaryPage(pdf_url);
|
||||
ASSERT_EQ(web_contents->GetURL(), pdf_url);
|
||||
}
|
||||
|
||||
class PDFExtensionUnseasonedTest
|
||||
: public PDFExtensionTestWithTestGuestViewManager {
|
||||
protected:
|
||||
static extensions::StreamContainer* GetStreamContainer(
|
||||
WebContents* guest_contents) {
|
||||
extensions::MimeHandlerViewGuest* guest =
|
||||
extensions::MimeHandlerViewGuest::FromWebContents(guest_contents);
|
||||
if (!guest) {
|
||||
ADD_FAILURE() << "No MimeHandlerViewGuest";
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
extensions::StreamContainer* container = guest->GetStreamWeakPtr().get();
|
||||
EXPECT_TRUE(container);
|
||||
return container;
|
||||
}
|
||||
|
||||
// Loads a PDF viewer's guest `WebContents`. Unlike `EnsurePDFHasLoaded()`,
|
||||
// this does not require that the PDF viewer loads completely, which is useful
|
||||
// for testing the (currently) incomplete unseasoned PDF viewer.
|
||||
WebContents* LoadGuestContentsOnly() {
|
||||
if (!ui_test_utils::NavigateToURL(
|
||||
browser(), embedded_test_server()->GetURL("/pdf/test.pdf"))) {
|
||||
ADD_FAILURE() << "Initial navigation failed";
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
WebContents* guest_contents =
|
||||
GetGuestViewManager()->WaitForSingleGuestCreated();
|
||||
if (!guest_contents) {
|
||||
ADD_FAILURE() << "No guest WebContents";
|
||||
return nullptr;
|
||||
}
|
||||
|
||||
WaitForLoadStart(guest_contents);
|
||||
EXPECT_TRUE(content::WaitForLoadStop(guest_contents));
|
||||
return guest_contents;
|
||||
}
|
||||
};
|
||||
|
||||
class PDFExtensionUnseasonedDisabledTest : public PDFExtensionUnseasonedTest {
|
||||
protected:
|
||||
std::vector<base::Feature> GetDisabledFeatures() const override {
|
||||
std::vector<base::Feature> disabled =
|
||||
PDFExtensionUnseasonedTest::GetDisabledFeatures();
|
||||
disabled.push_back(chrome_pdf::features::kPdfUnseasoned);
|
||||
return disabled;
|
||||
}
|
||||
};
|
||||
|
||||
class PDFExtensionUnseasonedEnabledTest : public PDFExtensionUnseasonedTest {
|
||||
protected:
|
||||
std::vector<base::Feature> GetEnabledFeatures() const override {
|
||||
std::vector<base::Feature> enabled =
|
||||
PDFExtensionUnseasonedTest::GetEnabledFeatures();
|
||||
enabled.push_back(chrome_pdf::features::kPdfUnseasoned);
|
||||
return enabled;
|
||||
}
|
||||
};
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PDFExtensionUnseasonedDisabledTest,
|
||||
StreamLoaderRegisteredAsSubresource) {
|
||||
WebContents* guest_contents = LoadGuestContentsOnly();
|
||||
ASSERT_TRUE(guest_contents);
|
||||
|
||||
extensions::StreamContainer* container = GetStreamContainer(guest_contents);
|
||||
ASSERT_TRUE(container);
|
||||
|
||||
EXPECT_FALSE(container->TakeTransferrableURLLoader());
|
||||
}
|
||||
|
||||
IN_PROC_BROWSER_TEST_F(PDFExtensionUnseasonedEnabledTest,
|
||||
StreamLoaderNotRegisteredAsSubresource) {
|
||||
WebContents* guest_contents = LoadGuestContentsOnly();
|
||||
ASSERT_TRUE(guest_contents);
|
||||
|
||||
extensions::StreamContainer* container = GetStreamContainer(guest_contents);
|
||||
ASSERT_TRUE(container);
|
||||
|
||||
EXPECT_TRUE(container->TakeTransferrableURLLoader());
|
||||
}
|
||||
|
@ -520,6 +520,8 @@ source_set("browser_sources") {
|
||||
"//extensions/strings",
|
||||
"//google_apis",
|
||||
"//net",
|
||||
"//pdf:buildflags",
|
||||
"//pdf:features",
|
||||
"//ppapi/buildflags",
|
||||
"//services/data_decoder/public/cpp:cpp",
|
||||
"//services/device/public/cpp/hid",
|
||||
|
@ -28,6 +28,8 @@ include_rules = [
|
||||
"+net/url_request/redirect_util.h",
|
||||
# Needed for a flag.
|
||||
"+net/url_request/url_request.h",
|
||||
"+pdf/buildflags.h",
|
||||
"+pdf/pdf_features.h",
|
||||
# This directory contains build flags and does not pull all of PPAPI in.
|
||||
"+ppapi/buildflags",
|
||||
"+services/data_decoder/public",
|
||||
|
@ -7,6 +7,7 @@
|
||||
#include <utility>
|
||||
|
||||
#include "base/bind.h"
|
||||
#include "base/feature_list.h"
|
||||
#include "components/guest_view/common/guest_view_constants.h"
|
||||
#include "content/public/browser/host_zoom_map.h"
|
||||
#include "content/public/browser/navigation_handle.h"
|
||||
@ -31,12 +32,17 @@
|
||||
#include "extensions/common/mojom/guest_view.mojom.h"
|
||||
#include "extensions/strings/grit/extensions_strings.h"
|
||||
#include "mojo/public/cpp/bindings/associated_remote.h"
|
||||
#include "pdf/buildflags.h"
|
||||
#include "services/service_manager/public/cpp/binder_registry.h"
|
||||
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
|
||||
#include "third_party/blink/public/common/input/web_gesture_event.h"
|
||||
#include "third_party/blink/public/common/web_preferences/web_preferences.h"
|
||||
#include "third_party/blink/public/mojom/frame/frame_owner_element_type.mojom.h"
|
||||
|
||||
#if BUILDFLAG(ENABLE_PDF)
|
||||
#include "pdf/pdf_features.h"
|
||||
#endif // BUILDFLAG(ENABLE_PDF)
|
||||
|
||||
using content::WebContents;
|
||||
using guest_view::GuestViewBase;
|
||||
|
||||
@ -440,6 +446,18 @@ void MimeHandlerViewGuest::DocumentOnLoadCompletedInMainFrame(
|
||||
|
||||
void MimeHandlerViewGuest::ReadyToCommitNavigation(
|
||||
content::NavigationHandle* navigation_handle) {
|
||||
#if BUILDFLAG(ENABLE_PDF)
|
||||
if (base::FeatureList::IsEnabled(chrome_pdf::features::kPdfUnseasoned)) {
|
||||
const GURL& url = navigation_handle->GetURL();
|
||||
if (url.SchemeIs(kExtensionScheme) &&
|
||||
url.host_piece() == extension_misc::kPdfExtensionId) {
|
||||
// The unseasoned PDF viewer will navigate to the stream URL (using
|
||||
// PdfNavigtionThrottle), rather than using it as a subresource.
|
||||
return;
|
||||
}
|
||||
}
|
||||
#endif // BUILDFLAG(ENABLE_PDF)
|
||||
|
||||
navigation_handle->RegisterSubresourceOverride(
|
||||
stream_->TakeTransferrableURLLoader());
|
||||
}
|
||||
|
@ -22,6 +22,13 @@ const base::Feature kPdfIncrementalLoading = {
|
||||
const base::Feature kPdfPartialLoading = {"PdfPartialLoading",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT};
|
||||
|
||||
// This feature eventually will replace the ENABLE_PDF_UNSEASONED build flag.
|
||||
// Code paths should be guarded by this feature or the build flag, not both.
|
||||
// TODO(crbug.com/1141953): Use this to back a chrome://flags entry when ready.
|
||||
// TODO(crbug.com/702993): Remove this once the PDF viewer is Pepper-free.
|
||||
const base::Feature kPdfUnseasoned = {"PdfUnseasoned",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT};
|
||||
|
||||
// Feature has no effect if Chrome is built with no XFA support.
|
||||
const base::Feature kPdfXfaSupport = {"PdfXfaSupport",
|
||||
base::FEATURE_DISABLED_BY_DEFAULT};
|
||||
|
@ -16,6 +16,7 @@ namespace features {
|
||||
extern const base::Feature kAccessiblePDFForm;
|
||||
extern const base::Feature kPdfIncrementalLoading;
|
||||
extern const base::Feature kPdfPartialLoading;
|
||||
extern const base::Feature kPdfUnseasoned;
|
||||
extern const base::Feature kPdfXfaSupport;
|
||||
extern const base::Feature kTabAcrossPDFAnnotations;
|
||||
|
||||
|
Reference in New Issue
Block a user