Skip to content

Commit

Permalink
Revert "printing: Mojofy PrintMsg_* in PrintViewManagerBase"
Browse files Browse the repository at this point in the history
This reverts commit 10d377b.

Reason for revert: Speculative revert for webview_instrumentation_test_apk failing on https://ci.chromium.org/p/chromium/builders/ci/Android%20CFI/6755?blamelist=1#blamelist-tab

Original change's description:
> printing: Mojofy PrintMsg_* in PrintViewManagerBase
> 
> Replace PrintMsg_PrintingDone, PrintMsg_PrintPages, and
> PrintMsg_SetPrintingEnabled IPC messages with new Mojo methods. Update
> AwPrintManager and HeadlessPrintManager to also use these new methods.
> 
> Bug: 1008939
> Test: Print a webpage and PDF
> Change-Id: I3a403dd47ffc9c4debe819d26b00eccbd24b1180
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1876473
> Commit-Queue: Jesse Schettler <jschettler@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Lei Zhang <thestig@chromium.org>
> Reviewed-by: Changwan Ryu <changwan@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#710576}

TBR=sky@chromium.org,dcheng@chromium.org,thestig@chromium.org,changwan@chromium.org,jschettler@chromium.org

Change-Id: Id6c6676aafab9e6abeb8d99bc18613a6ba5a2c60
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1008939
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1890070
Reviewed-by: Kamila Hasanbega <hkamila@google.com>
Commit-Queue: Kamila Hasanbega <hkamila@google.com>
Cr-Commit-Position: refs/heads/master@{#710719}
  • Loading branch information
hkamila authored and Commit Bot committed Oct 30, 2019
1 parent a6d1c54 commit 5bb59ab
Show file tree
Hide file tree
Showing 14 changed files with 99 additions and 90 deletions.
1 change: 0 additions & 1 deletion android_webview/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ source_set("browser") {
"//components/prefs",
"//components/printing/browser",
"//components/printing/common",
"//components/printing/common:mojo_interfaces",
"//components/safe_browsing",
"//components/safe_browsing:features",
"//components/safe_browsing:ping_manager",
Expand Down
3 changes: 1 addition & 2 deletions android_webview/browser/aw_print_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,7 @@ void AwPrintManager::PdfWritingDone(int page_count) {
bool AwPrintManager::PrintNow() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
auto* rfh = web_contents()->GetMainFrame();
GetPrintRenderFrame(rfh)->PrintRequestedPages();
return true;
return rfh->Send(new PrintMsg_PrintPages(rfh->GetRoutingID()));
}

void AwPrintManager::OnGetDefaultPrintSettings(
Expand Down
17 changes: 17 additions & 0 deletions chrome/browser/printing/print_view_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include "chrome/browser/printing/print_preview_dialog_controller.h"
#include "chrome/browser/ui/webui/print_preview/print_preview_ui.h"
#include "chrome/common/chrome_content_client.h"
#include "components/printing/common/print.mojom.h"
#include "components/printing/common/print_messages.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/plugin_service.h"
Expand All @@ -26,6 +27,7 @@
#include "content/public/common/webplugininfo.h"
#include "ipc/ipc_message_macros.h"
#include "printing/buildflags/buildflags.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"

using content::BrowserThread;

Expand Down Expand Up @@ -192,6 +194,21 @@ void PrintViewManager::RenderFrameDeleted(
PrintViewManagerBase::RenderFrameDeleted(render_frame_host);
}

const mojo::AssociatedRemote<printing::mojom::PrintRenderFrame>&
PrintViewManager::GetPrintRenderFrame(content::RenderFrameHost* rfh) {
if (!print_render_frame_.is_bound()) {
rfh->GetRemoteAssociatedInterfaces()->GetInterface(&print_render_frame_);
print_render_frame_.set_disconnect_handler(
base::BindOnce(&PrintViewManager::OnPrintRenderFrameDisconnected,
base::Unretained(this)));
}
return print_render_frame_;
}

void PrintViewManager::OnPrintRenderFrameDisconnected() {
print_render_frame_.reset();
}

bool PrintViewManager::PrintPreview(
content::RenderFrameHost* rfh,
mojom::PrintRendererAssociatedPtrInfo print_renderer,
Expand Down
14 changes: 14 additions & 0 deletions chrome/browser/printing/print_view_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "chrome/browser/printing/print_view_manager_base.h"
#include "components/printing/common/print.mojom.h"
#include "content/public/browser/web_contents_user_data.h"
#include "mojo/public/cpp/bindings/associated_remote.h"
#include "printing/buildflags/buildflags.h"

namespace content {
Expand Down Expand Up @@ -79,6 +80,15 @@ class PrintViewManager : public PrintViewManagerBase,

struct FrameDispatchHelper;

// Helper method to fetch the PrintRenderFrame associated remote interface
// pointer.
const mojo::AssociatedRemote<printing::mojom::PrintRenderFrame>&
GetPrintRenderFrame(content::RenderFrameHost* rfh);

// Resets the PrintRenderFrame associated remote when it's disconnected from
// its receiver.
void OnPrintRenderFrameDisconnected();

// Helper method for PrintPreviewNow() and PrintPreviewWithRenderer().
// Initiate print preview of the current document by first notifying the
// renderer. Since this happens asynchronously, the print preview dialog
Expand Down Expand Up @@ -117,6 +127,10 @@ class PrintViewManager : public PrintViewManagerBase,
// flag is true between PrintForSystemDialogNow() and PrintPreviewDone().
bool is_switching_to_system_dialog_ = false;

// Used to transmit mojom interface method calls to the PrintRenderFrame
// associated remote.
mojo::AssociatedRemote<printing::mojom::PrintRenderFrame> print_render_frame_;

WEB_CONTENTS_USER_DATA_KEY_DECL();

DISALLOW_COPY_AND_ASSIGN(PrintViewManager);
Expand Down
17 changes: 8 additions & 9 deletions chrome/browser/printing/print_view_manager_base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,9 @@ PrintViewManagerBase::~PrintViewManagerBase() {
bool PrintViewManagerBase::PrintNow(content::RenderFrameHost* rfh) {
DisconnectFromCurrentPrintJob();

// Don't print / print preview interstitials or crashed tabs.
if (IsInterstitialOrCrashed())
return false;

SetPrintingRFH(rfh);
GetPrintRenderFrame(rfh)->PrintRequestedPages();
return true;
int32_t id = rfh->GetRoutingID();
return PrintNowInternal(rfh, std::make_unique<PrintMsg_PrintPages>(id));
}

#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
Expand Down Expand Up @@ -615,8 +611,11 @@ void PrintViewManagerBase::ReleasePrintJob() {
if (!print_job_)
return;

if (rfh)
GetPrintRenderFrame(rfh)->PrintingDone(printing_succeeded_);
if (rfh) {
auto msg = std::make_unique<PrintMsg_PrintingDone>(rfh->GetRoutingID(),
printing_succeeded_);
rfh->Send(msg.release());
}

registrar_.Remove(this, chrome::NOTIFICATION_PRINT_JOB_EVENT,
content::Source<PrintJob>(print_job_.get()));
Expand Down Expand Up @@ -724,7 +723,7 @@ void PrintViewManagerBase::ReleasePrinterQuery() {

void PrintViewManagerBase::SendPrintingEnabled(bool enabled,
content::RenderFrameHost* rfh) {
GetPrintRenderFrame(rfh)->SetPrintingEnabled(enabled);
rfh->Send(new PrintMsg_SetPrintingEnabled(rfh->GetRoutingID(), enabled));
}

} // namespace printing
1 change: 0 additions & 1 deletion components/printing/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ static_library("browser") {
"//components/crash/core/common",
"//components/discardable_memory/service",
"//components/printing/common",
"//components/printing/common:mojo_interfaces",
"//components/services/pdf_compositor/public/cpp",
"//components/services/pdf_compositor/public/mojom",
"//components/strings:components_strings_grit",
Expand Down
15 changes: 0 additions & 15 deletions components/printing/browser/print_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@

#include "components/printing/browser/print_manager.h"

#include "base/bind.h"
#include "build/build_config.h"
#include "components/printing/common/print_messages.h"
#include "content/public/browser/render_frame_host.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"

namespace printing {

Expand Down Expand Up @@ -112,19 +110,6 @@ void PrintManager::OnPrintingFailed(int cookie) {
#endif
}

const mojo::AssociatedRemote<printing::mojom::PrintRenderFrame>&
PrintManager::GetPrintRenderFrame(content::RenderFrameHost* rfh) {
// When print preview is closed, the remote is disconnected from the receiver.
// Reset a disconnected remote before using it again.
if (print_render_frame_.is_bound() && !print_render_frame_.is_connected())
print_render_frame_.reset();

if (!print_render_frame_.is_bound())
rfh->GetRemoteAssociatedInterfaces()->GetInterface(&print_render_frame_);

return print_render_frame_;
}

void PrintManager::PrintingRenderFrameDeleted() {
#if defined(OS_ANDROID)
PdfWritingDone(0);
Expand Down
11 changes: 0 additions & 11 deletions components/printing/browser/print_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@

#include "base/macros.h"
#include "build/build_config.h"
#include "components/printing/common/print.mojom.h"
#include "content/public/browser/web_contents_observer.h"
#include "mojo/public/cpp/bindings/associated_remote.h"

#if defined(OS_ANDROID)
#include "base/callback.h"
Expand Down Expand Up @@ -41,11 +39,6 @@ class PrintManager : public content::WebContentsObserver {
protected:
explicit PrintManager(content::WebContents* contents);

// Helper method to fetch the PrintRenderFrame associated remote interface
// pointer.
const mojo::AssociatedRemote<printing::mojom::PrintRenderFrame>&
GetPrintRenderFrame(content::RenderFrameHost* rfh);

// Terminates or cancels the print job if one was pending.
void PrintingRenderFrameDeleted();

Expand Down Expand Up @@ -103,10 +96,6 @@ class PrintManager : public content::WebContentsObserver {
private:
void OnDidGetDocumentCookie(int cookie);

// Used to transmit mojom interface method calls to the PrintRenderFrame
// associated remote.
mojo::AssociatedRemote<printing::mojom::PrintRenderFrame> print_render_frame_;

DISALLOW_COPY_AND_ASSIGN(PrintManager);
};

Expand Down
10 changes: 0 additions & 10 deletions components/printing/common/print.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@ interface PrintRenderer {
// Render process interface exposed to the browser to handle most of the
// printing grunt work for RenderView.
interface PrintRenderFrame {
// Tells the RenderFrame to switch the CSS to print media type, render every
// requested page, and then switch back the CSS to display media type.
PrintRequestedPages();

// Tells the RenderFrame to switch the CSS to print media type, render every
// requested page using the print preview document's frame/node, and then
// switch the CSS back to display media type.
Expand All @@ -32,10 +28,4 @@ interface PrintRenderFrame {
// Tells the RenderFrame that the print preview dialog was closed.
[EnableIf=enable_print_preview]
OnPrintPreviewDialogClosed();

// Tells the RenderFrame whether printing is enabled or not.
SetPrintingEnabled(bool enabled);

// Tells the RenderFrame that printing is done so it can clean up.
PrintingDone(bool success);
};
13 changes: 13 additions & 0 deletions components/printing/common/print_messages.h
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,22 @@ IPC_STRUCT_END()
// node, depending on which mode the RenderFrame is in.
IPC_MESSAGE_ROUTED0(PrintMsg_PrintNodeUnderContextMenu)

#if BUILDFLAG(ENABLE_PRINTING)
// Tells the RenderFrame to switch the CSS to print media type, renders every
// requested pages and switch back the CSS to display media type.
IPC_MESSAGE_ROUTED0(PrintMsg_PrintPages)
#endif

// Print content of an out-of-process subframe.
IPC_MESSAGE_ROUTED1(PrintMsg_PrintFrameContent, PrintMsg_PrintFrame_Params)

// Tells the RenderFrame that printing is done so it can clean up.
IPC_MESSAGE_ROUTED1(PrintMsg_PrintingDone,
bool /* success */)

// Tells the RenderFrame whether printing is enabled or not.
IPC_MESSAGE_ROUTED1(PrintMsg_SetPrintingEnabled, bool /* enabled */)

#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
// Tells the RenderFrame to switch the CSS to print media type, renders every
// requested pages for print preview using the given |settings|. This gets
Expand Down
63 changes: 33 additions & 30 deletions components/printing/renderer/print_render_frame_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1174,10 +1174,13 @@ bool PrintRenderFrameHelper::OnMessageReceived(const IPC::Message& message) {

bool handled = true;
IPC_BEGIN_MESSAGE_MAP(PrintRenderFrameHelper, message)
IPC_MESSAGE_HANDLER(PrintMsg_PrintPages, OnPrintPages)
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
IPC_MESSAGE_HANDLER(PrintMsg_PrintPreview, OnPrintPreview)
IPC_MESSAGE_HANDLER(PrintMsg_PrintingDone, OnPrintingDone)
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
IPC_MESSAGE_HANDLER(PrintMsg_PrintFrameContent, OnPrintFrameContent)
IPC_MESSAGE_HANDLER(PrintMsg_SetPrintingEnabled, OnSetPrintingEnabled)
IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP()

Expand All @@ -1197,27 +1200,6 @@ void PrintRenderFrameHelper::BindPrintRenderFrameReceiver(
receivers_.Add(this, std::move(receiver));
}

void PrintRenderFrameHelper::PrintRequestedPages() {
ScopedIPC scoped_ipc(weak_ptr_factory_.GetWeakPtr());
if (ipc_nesting_level_ > 1)
return;

blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
frame->DispatchBeforePrintEvent();
// Don't print if the RenderFrame is gone.
if (render_frame_gone_)
return;

// If we are printing a PDF extension frame, find the plugin node and print
// that instead.
auto plugin = delegate_->GetPdfElement(frame);
Print(frame, plugin, PrintRequestType::kRegular);
if (!render_frame_gone_)
frame->DispatchAfterPrintEvent();
// WARNING: |this| may be gone at this point. Do not do any more work here and
// just return.
}

void PrintRenderFrameHelper::PrintForSystemDialog() {
ScopedIPC scoped_ipc(weak_ptr_factory_.GetWeakPtr());
if (ipc_nesting_level_ > 1)
Expand All @@ -1227,9 +1209,10 @@ void PrintRenderFrameHelper::PrintForSystemDialog() {
NOTREACHED();
return;
}
auto weak_this = weak_ptr_factory_.GetWeakPtr();
Print(frame, print_preview_context_.source_node(),
PrintRequestType::kRegular);
if (!render_frame_gone_)
if (weak_this)
frame->DispatchAfterPrintEvent();
// WARNING: |this| may be gone at this point. Do not do any more work here and
// just return.
Expand Down Expand Up @@ -1267,17 +1250,24 @@ void PrintRenderFrameHelper::OnPrintPreviewDialogClosed() {
}
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)

void PrintRenderFrameHelper::PrintingDone(bool success) {
ScopedIPC scoped_ipc(weak_ptr_factory_.GetWeakPtr());
void PrintRenderFrameHelper::OnPrintPages() {
if (ipc_nesting_level_ > 1)
return;
notify_browser_of_print_failure_ = false;
DidFinishPrinting(success ? OK : FAIL_PRINT);
}

void PrintRenderFrameHelper::SetPrintingEnabled(bool enabled) {
ScopedIPC scoped_ipc(weak_ptr_factory_.GetWeakPtr());
is_printing_enabled_ = enabled;
auto weak_this = weak_ptr_factory_.GetWeakPtr();
blink::WebLocalFrame* frame = render_frame()->GetWebFrame();
frame->DispatchBeforePrintEvent();
if (!weak_this)
return;

// If we are printing a PDF extension frame, find the plugin node and print
// that instead.
auto plugin = delegate_->GetPdfElement(frame);
Print(frame, plugin, PrintRequestType::kRegular);
if (weak_this)
frame->DispatchAfterPrintEvent();
// WARNING: |this| may be gone at this point. Do not do any more work here and
// just return.
}

void PrintRenderFrameHelper::GetPageSizeAndContentAreaFromPageLayout(
Expand Down Expand Up @@ -1568,6 +1558,19 @@ int PrintRenderFrameHelper::GetFitToPageScaleFactor(
}
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)

void PrintRenderFrameHelper::OnPrintingDone(bool success) {
if (ipc_nesting_level_ > 1)
return;
notify_browser_of_print_failure_ = false;
if (!success)
LOG(ERROR) << "Failure in OnPrintingDone";
DidFinishPrinting(success ? OK : FAIL_PRINT);
}

void PrintRenderFrameHelper::OnSetPrintingEnabled(bool enabled) {
is_printing_enabled_ = enabled;
}

void PrintRenderFrameHelper::OnPrintFrameContent(
const PrintMsg_PrintFrame_Params& params) {
if (ipc_nesting_level_ > 1)
Expand Down
11 changes: 6 additions & 5 deletions components/printing/renderer/print_render_frame_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ class PrintRenderFrameHelper
friend class PrintRenderFrameHelperTestBase;
FRIEND_TEST_ALL_PREFIXES(MAYBE_PrintRenderFrameHelperPreviewTest,
BlockScriptInitiatedPrinting);
FRIEND_TEST_ALL_PREFIXES(MAYBE_PrintRenderFrameHelperTest,
PrintRequestedPages);
FRIEND_TEST_ALL_PREFIXES(MAYBE_PrintRenderFrameHelperTest, OnPrintPages);
FRIEND_TEST_ALL_PREFIXES(MAYBE_PrintRenderFrameHelperTest,
BlockScriptInitiatedPrinting);
FRIEND_TEST_ALL_PREFIXES(MAYBE_PrintRenderFrameHelperTest,
Expand Down Expand Up @@ -212,22 +211,21 @@ class PrintRenderFrameHelper
mojo::PendingAssociatedReceiver<mojom::PrintRenderFrame> receiver);

// printing::mojom::PrintRenderFrame:
void PrintRequestedPages() override;
void PrintForSystemDialog() override;
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
void InitiatePrintPreview(
mojom::PrintRendererAssociatedPtrInfo print_renderer,
bool has_selection) override;
void OnPrintPreviewDialogClosed() override;
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
void PrintingDone(bool success) override;
void SetPrintingEnabled(bool enabled) override;

// Message handlers ---------------------------------------------------------
void OnPrintPages();
#if BUILDFLAG(ENABLE_PRINT_PREVIEW)
void OnPrintPreview(const base::DictionaryValue& settings);
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
void OnPrintFrameContent(const PrintMsg_PrintFrame_Params& params);
void OnPrintingDone(bool success);

// Get |page_size| and |content_area| information from
// |page_layout_in_points|.
Expand Down Expand Up @@ -260,6 +258,9 @@ class PrintRenderFrameHelper
int GetFitToPageScaleFactor(const gfx::Rect& printable_area_in_points);
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)

// Enable/Disable printing.
void OnSetPrintingEnabled(bool enabled);

// Main printing code -------------------------------------------------------

// Print with the system dialog.
Expand Down
Loading

0 comments on commit 5bb59ab

Please sign in to comment.