Skip to content

Commit b37e479

Browse files
Sadrul ChowdhuryChromium LUCI CQ
authored andcommitted
Revert "[pdf] Change IsPdfExtensionUrl() to IsPdfExtensionOrigin()"
This reverts commit db57e30. Reason for revert: breaks builds in several perf bots https://ci.chromium.org/ui/p/chrome/builders/ci/linux-builder-perf/349623/overview Original change's description: > [pdf] Change IsPdfExtensionUrl() to IsPdfExtensionOrigin() > > The check really pertains to the origin, not the entire URL. It also > allows for a merging of logic between IsPdfExtensionOrigin() and > IsPdfInternalPluginAllowedOrigin(). Furthermore, it makes the function > more appropriate for security checks. > > Use IsPdfInternalPluginAllowedOrigin() in the implementation of > ChromePrintRenderFrameHelperDelegate::GetPdfElement(). > > Change-Id: Ia8a6bb99a9e2d19b8bc857525ac74ac93e29c565 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3200416 > Commit-Queue: Daniel Hosseinian <[email protected]> > Reviewed-by: Lei Zhang <[email protected]> > Cr-Commit-Position: refs/heads/main@{#927504} NOTREECHECKS Change-Id: Id8f4ceb05d79e5f93af6e42aaaae8398103b3ed8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3201368 Bot-Commit: Rubber Stamper <[email protected]> Reviewed-by: Scott Violet <[email protected]> Reviewed-by: Johann Koenig <[email protected]> Commit-Queue: Johann Koenig <[email protected]> Owners-Override: Johann Koenig <[email protected]> Cr-Commit-Position: refs/heads/main@{#927565}
1 parent 612a943 commit b37e479

File tree

5 files changed

+16
-11
lines changed

5 files changed

+16
-11
lines changed

chrome/browser/chrome_content_browser_client.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2920,7 +2920,7 @@ bool UpdatePreferredColorScheme(WebPreferences* web_prefs,
29202920
if (!base::FeatureList::IsEnabled(features::kWebUIDarkMode)) {
29212921
// Update based on last committed url.
29222922
force_light = force_light || url.SchemeIs(content::kChromeUIScheme);
2923-
force_light = force_light || IsPdfExtensionOrigin(url::Origin::Create(url));
2923+
force_light = force_light || IsPdfExtensionUrl(url);
29242924
}
29252925

29262926
// Reauth WebUI doesn't support dark mode yet because it shares the dialog

chrome/browser/renderer_context_menu/render_view_context_menu.cc

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2269,8 +2269,7 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
22692269
// Rotate commands should be disabled when in PDF Viewer's Presentation
22702270
// mode.
22712271
is_pdf_viewer_fullscreen =
2272-
IsPdfExtensionOrigin(url::Origin::Create(GetDocumentURL(params_))) &&
2273-
IsHTML5Fullscreen();
2272+
IsPdfExtensionUrl(GetDocumentURL(params_)) && IsHTML5Fullscreen();
22742273
#endif
22752274
return !is_pdf_viewer_fullscreen &&
22762275
(params_.media_flags & ContextMenuData::kMediaCanRotate) != 0;

chrome/common/pdf_util.cc

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,23 @@ std::string GetPDFPlaceholderHTML(const GURL& pdf_url) {
3939
return webui::GetI18nTemplateHtml(template_html, &values);
4040
}
4141

42-
bool IsPdfExtensionOrigin(const url::Origin& origin) {
42+
bool IsPdfExtensionUrl(const GURL& url) {
4343
#if BUILDFLAG(ENABLE_EXTENSIONS)
44-
return origin.scheme() == extensions::kExtensionScheme &&
45-
origin.host() == extension_misc::kPdfExtensionId;
44+
return url.SchemeIs(extensions::kExtensionScheme) &&
45+
url.host_piece() == extension_misc::kPdfExtensionId;
4646
#else
4747
return false;
4848
#endif
4949
}
5050

5151
bool IsPdfInternalPluginAllowedOrigin(const url::Origin& origin) {
52-
if (IsPdfExtensionOrigin(origin))
52+
#if BUILDFLAG(ENABLE_EXTENSIONS)
53+
// Allow embedding the internal PDF plugin in the built-in PDF extension.
54+
if (origin.scheme() == extensions::kExtensionScheme &&
55+
origin.host() == extension_misc::kPdfExtensionId) {
5356
return true;
57+
}
58+
#endif // BUILDFLAG(ENABLE_EXTENSIONS)
5459

5560
// Allow embedding the internal PDF plugin in chrome://print.
5661
if (origin == url::Origin::Create(GURL(chrome::kChromeUIPrintURL)))

chrome/common/pdf_util.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ void ReportPDFLoadStatus(PDFLoadStatus status);
3030
// Returns the HTML contents of the placeholder.
3131
std::string GetPDFPlaceholderHTML(const GURL& pdf_url);
3232

33-
// Returns whether `origin` is for the built-in PDF extension.
34-
bool IsPdfExtensionOrigin(const url::Origin& origin);
33+
// Returns `true` if `url` is for the built-in PDF extension.
34+
bool IsPdfExtensionUrl(const GURL& url);
3535

3636
// Returns `true` if the origin is allowed to create the internal PDF plugin.
3737
// Note that for the Pepper-free plugin, this applies to the origin of the

chrome/renderer/printing/chrome_print_render_frame_helper_delegate.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@
99
#include "base/command_line.h"
1010
#include "base/strings/string_util.h"
1111
#include "chrome/common/chrome_switches.h"
12+
#include "chrome/common/url_constants.h"
1213
#include "content/public/renderer/render_frame.h"
1314
#include "extensions/buildflags/buildflags.h"
1415
#include "mojo/public/cpp/bindings/remote.h"
1516
#include "third_party/blink/public/common/browser_interface_broker_proxy.h"
1617
#include "third_party/blink/public/web/web_document.h"
1718
#include "third_party/blink/public/web/web_element.h"
1819
#include "third_party/blink/public/web/web_local_frame.h"
19-
#include "url/origin.h"
2020

2121
#if BUILDFLAG(ENABLE_EXTENSIONS)
2222
#include "chrome/common/pdf_util.h"
@@ -33,7 +33,8 @@ ChromePrintRenderFrameHelperDelegate::~ChromePrintRenderFrameHelperDelegate() =
3333
blink::WebElement ChromePrintRenderFrameHelperDelegate::GetPdfElement(
3434
blink::WebLocalFrame* frame) {
3535
#if BUILDFLAG(ENABLE_EXTENSIONS)
36-
if (IsPdfInternalPluginAllowedOrigin(frame->GetSecurityOrigin())) {
36+
GURL url = frame->GetDocument().Url();
37+
if (url.GetOrigin() == chrome::kChromeUIPrintURL || IsPdfExtensionUrl(url)) {
3738
// <object> with id="plugin" is created in
3839
// chrome/browser/resources/pdf/pdf_viewer_base.js.
3940
auto viewer_element = frame->GetDocument().GetElementById("viewer");

0 commit comments

Comments
 (0)