Skip to content

Commit

Permalink
[unseasoned-pdf] Print the correct RenderFrameHost using one WebContents
Browse files Browse the repository at this point in the history
GetRenderFrameHostToUse() took two WebContents to apply its heuristics
for printing the right RenderFrameHost. The function assumed the
WebContents to use were that of a full page plugin if and only if it was
different than the "owning" WebContents.

That logic was faulty when the "owning" WebContents was that of a full
page plugin already, which is possible when the print operation is
targeted to the plugin's WebContents from the context menu.

Instead, directly check whether the WebContents to use is a full page
plugin, and dispose of the need to check using the "owning" WebContents.

Fixed: 1259307
Change-Id: I36ee95f2385a457e18db43a24f853f9fd1bdbc1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3219937
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Hosseinian <dhoss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#931850}
  • Loading branch information
Daniel Hosseinian authored and Chromium LUCI CQ committed Oct 15, 2021
1 parent 9b241cc commit df5669f
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 14 deletions.
16 changes: 16 additions & 0 deletions chrome/browser/pdf/pdf_extension_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
#include "chrome/browser/ui/browser_commands.h"
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
#include "chrome/browser/printing/print_view_manager.h"
#include "ui/base/ui_base_types.h"
#else
#include "chrome/browser/printing/print_view_manager_basic.h"
#endif
Expand Down Expand Up @@ -1633,6 +1634,21 @@ IN_PROC_BROWSER_TEST_P(PDFExtensionTest, PrintCommand) {
chrome::Print(browser());
print_observer.WaitForPrintPreview();
}

IN_PROC_BROWSER_TEST_P(PDFExtensionTest, ContextMenuPrintCommand) {
content::WebContents* guest_contents =
LoadPdfGetGuestContents(embedded_test_server()->GetURL("/pdf/test.pdf"));
content::RenderFrameHost* plugin_frame = GetPluginFrame(guest_contents);
ASSERT_TRUE(plugin_frame);

// Executes the print command as soon as the context menu is shown.
ContextMenuNotificationObserver context_menu_observer(IDC_PRINT);

PrintObserver print_observer(guest_contents, plugin_frame);
guest_contents->GetMainFrame()->GetRenderWidgetHost()->ShowContextMenuAtPoint(
{1, 1}, ui::MENU_SOURCE_MOUSE);
print_observer.WaitForPrintPreview();
}
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
#endif // BUILDFLAG(ENABLE_PRINTING)

Expand Down
37 changes: 23 additions & 14 deletions chrome/browser/printing/print_view_manager_common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,35 +28,44 @@
namespace printing {

namespace {
// Returns true if `contents` is a full page MimeHandlerViewGuest plugin.
bool IsFullPagePlugin(content::WebContents* contents) {
#if BUILDFLAG(ENABLE_EXTENSIONS)
// Stores |guest_contents| in |result| and returns true if |guest_contents| is a
// full page MimeHandlerViewGuest plugin. Otherwise, returns false.
auto* guest_view =
extensions::MimeHandlerViewGuest::FromWebContents(contents);
if (guest_view && guest_view->is_full_page_plugin())
return true;
#endif
return false;
}

#if BUILDFLAG(ENABLE_EXTENSIONS)
// Stores `guest_contents` in `result` and returns true if
// `IsFullPagePlugin(guest_contents)`. Otherwise, returns false and `result`
// remains unchanged.
bool StoreFullPagePlugin(content::WebContents** result,
content::WebContents* guest_contents) {
auto* guest_view =
extensions::MimeHandlerViewGuest::FromWebContents(guest_contents);
if (guest_view && guest_view->is_full_page_plugin()) {
if (IsFullPagePlugin(guest_contents)) {
*result = guest_contents;
return true;
}
return false;
}
#endif // BUILDFLAG(ENABLE_EXTENSIONS)

// Pick the right RenderFrameHost based on the WebContentses.
// Pick the right RenderFrameHost based on the WebContents.
content::RenderFrameHost* GetRenderFrameHostToUse(
content::WebContents* original_contents,
content::WebContents* contents_to_use) {
if (original_contents == contents_to_use)
return GetFrameToPrint(contents_to_use);
content::WebContents* contents) {
if (!IsFullPagePlugin(contents))
return GetFrameToPrint(contents);

#if BUILDFLAG(ENABLE_PDF)
content::RenderFrameHost* pdf_rfh =
pdf_frame_util::FindPdfChildFrame(contents_to_use->GetMainFrame());
pdf_frame_util::FindPdfChildFrame(contents->GetMainFrame());
if (pdf_rfh)
return pdf_rfh;
#endif
return contents_to_use->GetMainFrame();
return contents->GetMainFrame();
}

} // namespace
Expand All @@ -79,7 +88,7 @@ void StartPrint(
return;

content::RenderFrameHost* rfh_to_use =
GetRenderFrameHostToUse(contents, contents_to_use);
GetRenderFrameHostToUse(contents_to_use);
if (!rfh_to_use)
return;

Expand Down Expand Up @@ -107,7 +116,7 @@ void StartBasicPrint(content::WebContents* contents) {
return;

content::RenderFrameHost* rfh_to_use =
GetRenderFrameHostToUse(contents, contents_to_use);
GetRenderFrameHostToUse(contents_to_use);
if (!rfh_to_use)
return;

Expand Down

0 comments on commit df5669f

Please sign in to comment.