Skip to content

Commit

Permalink
Make printing work better with OOPIF. (try 2)
Browse files Browse the repository at this point in the history
Fix failing Android WebView CTS tests this time.

BUG=631513,665909
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
TBR=nasko@chromium.org,rockot@chromium.org

Review-Url: https://codereview.chromium.org/2508923003
Cr-Commit-Position: refs/heads/master@{#432775}
  • Loading branch information
leizleiz authored and Commit bot committed Nov 17, 2016
1 parent db2ac24 commit cb959ce
Show file tree
Hide file tree
Showing 48 changed files with 521 additions and 353 deletions.
23 changes: 15 additions & 8 deletions android_webview/browser/aw_print_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "components/printing/browser/print_manager_utils.h"
#include "components/printing/common/print_messages.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_frame_host.h"

DEFINE_WEB_CONTENTS_USER_DATA_KEY(android_webview::AwPrintManager);

Expand Down Expand Up @@ -41,27 +42,33 @@ AwPrintManager::~AwPrintManager() {

bool AwPrintManager::PrintNow() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
return Send(new PrintMsg_PrintPages(routing_id()));
auto* rfh = web_contents()->GetMainFrame();
return rfh->Send(new PrintMsg_PrintPages(rfh->GetRoutingID()));
}

bool AwPrintManager::OnMessageReceived(const IPC::Message& message) {
bool AwPrintManager::OnMessageReceived(
const IPC::Message& message,
content::RenderFrameHost* render_frame_host) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(AwPrintManager, message)
IPC_MESSAGE_HANDLER_DELAY_REPLY(PrintHostMsg_GetDefaultPrintSettings,
OnGetDefaultPrintSettings)
IPC_BEGIN_MESSAGE_MAP_WITH_PARAM(AwPrintManager, message, render_frame_host)
IPC_MESSAGE_HANDLER_WITH_PARAM_DELAY_REPLY(
PrintHostMsg_GetDefaultPrintSettings, OnGetDefaultPrintSettings)
IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP()
return handled ? true : PrintManager::OnMessageReceived(message);
return handled ? true
: PrintManager::OnMessageReceived(message, render_frame_host);
}

void AwPrintManager::OnGetDefaultPrintSettings(IPC::Message* reply_msg) {
void AwPrintManager::OnGetDefaultPrintSettings(
content::RenderFrameHost* render_frame_host,
IPC::Message* reply_msg) {
// Unlike the printing_message_filter, we do process this in UI thread.
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
PrintMsg_Print_Params params;
printing::RenderParamsFromPrintSettings(settings_, &params);
params.document_cookie = cookie_;
PrintHostMsg_GetDefaultPrintSettings::WriteReplyParams(reply_msg, params);
Send(reply_msg);
render_frame_host->Send(reply_msg);
}

} // namespace android_webview
9 changes: 7 additions & 2 deletions android_webview/browser/aw_print_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,13 @@ class AwPrintManager : public printing::PrintManager,
const base::FileDescriptor& file_descriptor,
const PdfWritingDoneCallback& callback);

bool OnMessageReceived(const IPC::Message& message) override;
void OnGetDefaultPrintSettings(IPC::Message* reply_msg);
// printing::PrintManager:
bool OnMessageReceived(const IPC::Message& message,
content::RenderFrameHost* render_frame_host) override;

// IPC Handlers
void OnGetDefaultPrintSettings(content::RenderFrameHost* render_frame_host,
IPC::Message* reply_msg);

printing::PrintSettings settings_;

Expand Down
23 changes: 11 additions & 12 deletions android_webview/browser/aw_printing_message_filter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#include "base/file_descriptor_posix.h"
#include "components/printing/common/print_messages.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/render_view_host.h"
#include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h"

using content::BrowserThread;
Expand All @@ -18,15 +18,14 @@ namespace android_webview {

namespace {

AwPrintManager* GetPrintManager(int render_process_id, int render_view_id) {
AwPrintManager* GetPrintManager(int render_process_id, int render_frame_id) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
content::RenderViewHost* view = content::RenderViewHost::FromID(
render_process_id, render_view_id);
if (!view)
content::RenderFrameHost* frame =
content::RenderFrameHost::FromID(render_process_id, render_frame_id);
if (!frame)
return nullptr;
WebContents* web_contents = WebContents::FromRenderViewHost(view);
return web_contents ? AwPrintManager::FromWebContents(web_contents)
: nullptr;
WebContents* web_contents = WebContents::FromRenderFrameHost(frame);
return web_contents ? AwPrintManager::FromWebContents(web_contents) : nullptr;
}

} // namespace
Expand Down Expand Up @@ -61,12 +60,12 @@ bool AwPrintingMessageFilter::OnMessageReceived(const IPC::Message& message) {
}

void AwPrintingMessageFilter::OnAllocateTempFileForPrinting(
int render_view_id,
int render_frame_id,
base::FileDescriptor* temp_file_fd,
int* sequence_number) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
AwPrintManager* print_manager =
GetPrintManager(render_process_id_, render_view_id);
GetPrintManager(render_process_id_, render_frame_id);
if (!print_manager)
return;

Expand All @@ -76,11 +75,11 @@ void AwPrintingMessageFilter::OnAllocateTempFileForPrinting(
}

void AwPrintingMessageFilter::OnTempFileForPrintingWritten(
int render_view_id,
int render_frame_id,
int sequence_number) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
AwPrintManager* print_manager =
GetPrintManager(render_process_id_, render_view_id);
GetPrintManager(render_process_id_, render_frame_id);
if (print_manager)
print_manager->PdfWritingDone(true);
}
Expand Down
4 changes: 2 additions & 2 deletions android_webview/browser/aw_printing_message_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ class AwPrintingMessageFilter : public content::BrowserMessageFilter {

// Used to ask the browser allocate a temporary file for the renderer
// to fill in resulting PDF in renderer.
void OnAllocateTempFileForPrinting(int render_view_id,
void OnAllocateTempFileForPrinting(int render_frame_id,
base::FileDescriptor* temp_file_fd,
int* sequence_number);
void OnTempFileForPrintingWritten(int render_view_id, int sequence_number);
void OnTempFileForPrintingWritten(int render_frame_id, int sequence_number);

const int render_process_id_;

Expand Down
6 changes: 2 additions & 4 deletions android_webview/renderer/aw_content_renderer_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ void AwContentRendererClient::RenderFrameCreated(
content::RenderFrame* render_frame) {
new AwContentSettingsClient(render_frame);
new PrintRenderFrameObserver(render_frame);
new printing::PrintWebViewHelper(
render_frame, base::MakeUnique<AwPrintWebViewHelperDelegate>());
new AwRenderFrameExt(render_frame);

// TODO(jam): when the frame tree moves into content and parent() works at
Expand All @@ -173,10 +175,6 @@ void AwContentRendererClient::RenderViewCreated(
content::RenderView* render_view) {
AwRenderViewExt::RenderViewCreated(render_view);

new printing::PrintWebViewHelper(
render_view, std::unique_ptr<printing::PrintWebViewHelper::Delegate>(
new AwPrintWebViewHelperDelegate()));

#if BUILDFLAG(ENABLE_SPELLCHECK)
new SpellCheckProvider(render_view, spellcheck_.get());
#endif
Expand Down
5 changes: 2 additions & 3 deletions android_webview/renderer/aw_print_web_view_helper_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,10 @@

namespace android_webview {

AwPrintWebViewHelperDelegate::~AwPrintWebViewHelperDelegate(){
}
AwPrintWebViewHelperDelegate::~AwPrintWebViewHelperDelegate() {}

bool AwPrintWebViewHelperDelegate::CancelPrerender(
content::RenderView* render_view, int routing_id) {
content::RenderFrame* render_frame) {
return false;
}

Expand Down
8 changes: 5 additions & 3 deletions android_webview/renderer/aw_print_web_view_helper_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,16 @@ class AwPrintWebViewHelperDelegate
: public printing::PrintWebViewHelper::Delegate {
public:
~AwPrintWebViewHelperDelegate() override;
bool CancelPrerender(content::RenderView* render_view,
int routing_id) override;

private:
// printing::PrintWebViewHelper::Delegate:
bool CancelPrerender(content::RenderFrame* render_frame) override;
blink::WebElement GetPdfElement(blink::WebLocalFrame* frame) override;
bool IsPrintPreviewEnabled() override;
bool IsAskPrintSettingsEnabled() override;
bool IsScriptedPrintEnabled() override;
bool OverridePrint(blink::WebLocalFrame* frame) override;
}; // class AwPrintWebViewHelperDelegate
};

} // namespace android_webview

Expand Down
2 changes: 1 addition & 1 deletion android_webview/renderer/print_render_frame_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ bool PrintRenderFrameObserver::OnMessageReceived(

void PrintRenderFrameObserver::OnPrintNodeUnderContextMenu() {
printing::PrintWebViewHelper* helper =
printing::PrintWebViewHelper::Get(render_frame()->GetRenderView());
printing::PrintWebViewHelper::Get(render_frame());
if (helper)
helper->PrintNode(render_frame()->GetWebFrame()->contextMenuNode());
}
Expand Down
5 changes: 3 additions & 2 deletions chrome/browser/android/tab_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "chrome/browser/prerender/prerender_manager.h"
#include "chrome/browser/prerender/prerender_manager_factory.h"
#include "chrome/browser/printing/print_view_manager_basic.h"
#include "chrome/browser/printing/print_view_manager_common.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_android.h"
#include "chrome/browser/profiles/profile_manager.h"
Expand Down Expand Up @@ -669,10 +670,10 @@ bool TabAndroid::Print(JNIEnv* env, const JavaParamRef<jobject>& obj) {
printing::PrintViewManagerBasic::CreateForWebContents(web_contents());
printing::PrintViewManagerBasic* print_view_manager =
printing::PrintViewManagerBasic::FromWebContents(web_contents());
if (print_view_manager == NULL)
if (!print_view_manager)
return false;

print_view_manager->PrintNow();
print_view_manager->PrintNow(printing::GetFrameToPrint(web_contents()));
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ class RequestPrintPreviewObserver : public WebContentsObserver {

private:
// content::WebContentsObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override {
bool OnMessageReceived(const IPC::Message& message,
content::RenderFrameHost* render_frame_host) override {
IPC_BEGIN_MESSAGE_MAP(RequestPrintPreviewObserver, message)
IPC_MESSAGE_HANDLER(PrintHostMsg_RequestPrintPreview,
OnRequestPrintPreview)
Expand Down
28 changes: 17 additions & 11 deletions chrome/browser/printing/print_preview_dialog_controller_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ TEST_F(PrintPreviewDialogControllerUnitTest, GetOrCreatePreviewDialog) {
ASSERT_TRUE(dialog_controller);

// Get the preview dialog for initiator.
PrintViewManager::FromWebContents(initiator)->PrintPreviewNow(false);
PrintViewManager::FromWebContents(initiator)->PrintPreviewNow(
initiator->GetMainFrame(), false);
WebContents* preview_dialog =
dialog_controller->GetOrCreatePreviewDialog(initiator);

Expand Down Expand Up @@ -116,15 +117,17 @@ TEST_F(PrintPreviewDialogControllerUnitTest, MultiplePreviewDialogs) {
ASSERT_TRUE(dialog_controller);

// Create preview dialog for |web_contents_1|
PrintViewManager::FromWebContents(web_contents_1)->PrintPreviewNow(false);
PrintViewManager::FromWebContents(web_contents_1)
->PrintPreviewNow(web_contents_1->GetMainFrame(), false);
WebContents* preview_dialog_1 =
dialog_controller->GetOrCreatePreviewDialog(web_contents_1);

EXPECT_NE(web_contents_1, preview_dialog_1);
EXPECT_EQ(2, tab_strip_model->count());

// Create preview dialog for |web_contents_2|
PrintViewManager::FromWebContents(web_contents_2)->PrintPreviewNow(false);
PrintViewManager::FromWebContents(web_contents_2)
->PrintPreviewNow(web_contents_2->GetMainFrame(), false);
WebContents* preview_dialog_2 =
dialog_controller->GetOrCreatePreviewDialog(web_contents_2);

Expand Down Expand Up @@ -172,7 +175,8 @@ TEST_F(PrintPreviewDialogControllerUnitTest, ClearInitiatorDetails) {
ASSERT_TRUE(dialog_controller);

// Get the preview dialog for the initiator.
PrintViewManager::FromWebContents(initiator)->PrintPreviewNow(false);
PrintViewManager::FromWebContents(initiator)->PrintPreviewNow(
initiator->GetMainFrame(), false);
WebContents* preview_dialog =
dialog_controller->GetOrCreatePreviewDialog(initiator);

Expand Down Expand Up @@ -228,7 +232,7 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
WebContents* tiger_preview_dialog =
dialog_controller->GetOrCreatePreviewDialog(web_contents);
PrintViewManager* manager = PrintViewManager::FromWebContents(web_contents);
manager->PrintPreviewNow(false);
manager->PrintPreviewNow(web_contents->GetMainFrame(), false);

// New print preview dialog is a constrained window, so the number of tabs is
// still 1.
Expand All @@ -247,7 +251,7 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {

// Print preview now should return true as the navigation should have closed
// |tiger_preview_dialog| and the previous dialog should have closed.
EXPECT_TRUE(manager->PrintPreviewNow(false));
EXPECT_TRUE(manager->PrintPreviewNow(web_contents->GetMainFrame(), false));
WebContents* tiger_barb_preview_dialog =
dialog_controller->GetOrCreatePreviewDialog(web_contents);
ASSERT_TRUE(tiger_barb_preview_dialog);
Expand All @@ -261,13 +265,13 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
tiger_barb_preview_dialog);

// Now this returns false as |tiger_barb_preview_dialog| is open.
EXPECT_FALSE(manager->PrintPreviewNow(false));
EXPECT_FALSE(manager->PrintPreviewNow(web_contents->GetMainFrame(), false));

// Navigate with back button or ALT+LEFT ARROW to a similar page.
nav_controller.GoBack();
CommitPendingLoad(&nav_controller);
EXPECT_EQ(tiger, web_contents->GetLastCommittedURL());
EXPECT_TRUE(manager->PrintPreviewNow(false));
EXPECT_TRUE(manager->PrintPreviewNow(web_contents->GetMainFrame(), false));

// Get new dialog
WebContents* tiger_preview_dialog_2 =
Expand Down Expand Up @@ -296,7 +300,7 @@ TEST_F(PrintPreviewDialogControllerUnitTest, CloseDialogOnNavigation) {
// preview now should return false, dialog is still alive, and the dialog
// returned by GetOrCreatePreviewDialog should be the same as the earlier
// dialog.
EXPECT_FALSE(manager->PrintPreviewNow(false));
EXPECT_FALSE(manager->PrintPreviewNow(web_contents->GetMainFrame(), false));
EXPECT_FALSE(tiger_2_destroyed.dialog_destroyed());
WebContents* tiger_preview_dialog_2b =
dialog_controller->GetOrCreatePreviewDialog(web_contents);
Expand Down Expand Up @@ -326,7 +330,8 @@ TEST_F(PrintPreviewDialogControllerUnitTest, MultiplePreviewDialogsClose) {
ASSERT_TRUE(dialog_controller);

// Create preview dialog for |web_contents_1|. Should not create a new tab.
PrintViewManager::FromWebContents(web_contents_1)->PrintPreviewNow(false);
PrintViewManager::FromWebContents(web_contents_1)
->PrintPreviewNow(web_contents_1->GetMainFrame(), false);
WebContents* preview_dialog_1 =
dialog_controller->GetOrCreatePreviewDialog(web_contents_1);
EXPECT_NE(web_contents_1, preview_dialog_1);
Expand All @@ -339,7 +344,8 @@ TEST_F(PrintPreviewDialogControllerUnitTest, MultiplePreviewDialogsClose) {
EXPECT_EQ(2, tab_strip_model->count());

// Create preview dialog for |web_contents_2|
PrintViewManager::FromWebContents(web_contents_2)->PrintPreviewNow(false);
PrintViewManager::FromWebContents(web_contents_2)
->PrintPreviewNow(web_contents_2->GetMainFrame(), false);
WebContents* preview_dialog_2 =
dialog_controller->GetOrCreatePreviewDialog(web_contents_2);
EXPECT_NE(web_contents_2, preview_dialog_2);
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/printing/print_preview_message_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ void PrintPreviewMessageHandler::OnSetOptionsFromDocument(
}

bool PrintPreviewMessageHandler::OnMessageReceived(
const IPC::Message& message) {
const IPC::Message& message,
content::RenderFrameHost* render_frame_host) {
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(PrintPreviewMessageHandler, message)
IPC_MESSAGE_HANDLER(PrintHostMsg_RequestPrintPreview,
Expand Down
3 changes: 2 additions & 1 deletion chrome/browser/printing/print_preview_message_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ class PrintPreviewMessageHandler
~PrintPreviewMessageHandler() override;

// content::WebContentsObserver implementation.
bool OnMessageReceived(const IPC::Message& message) override;
bool OnMessageReceived(const IPC::Message& message,
content::RenderFrameHost* render_frame_host) override;

private:
explicit PrintPreviewMessageHandler(content::WebContents* web_contents);
Expand Down
Loading

0 comments on commit cb959ce

Please sign in to comment.