Skip to content

GitLab

T
tangled
Commit 87248e6a authored by Stephen McGruer's avatar Stephen McGruer Committed by Commit Bot
Browse files
Options

Revert "Narrow the scope of page pause when printing." 

This reverts commit f0f5ad44.

Reason for revert: Tentative revert for suspect of BackForwardCachePrintBrowserTest.DisableCaching failure; see https://crbug.com/1115515

If this does not resolve the issue, I will reland the CL (with apologies! :))

Original change's description:
> Narrow the scope of page pause when printing.
> 
> In the current implementation, sometimes the scope of a page's "pause"
> is too wide compared to the definition in the spec [1], and sometimes it
> does not pause at all. So narrow the scope to better conform to the
> spec, and apply it consistently to all possible ways users can print.
> 
> This is based on an earlier CL [2] which only did this for printing with
> the system dialog, but not for Print Preview. Some of the trickier cases
> handled, which require unpausing, are:
> - Navigations while previewing.
> - Printing selections, which require a navigation.
> 
> [1] https://html.spec.whatwg.org/C/#printing-steps
> [2] https://crrev.com/c/1669250
> 
> Bug: 956832
> Change-Id: I9bf274fcfb70eefe896fbbae9ce13a88b6f4dfc0
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2338819
> Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Commit-Queue: Lei Zhang <thestig@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#797120}

TBR=thestig@chromium.org,yukishiino@chromium.org,tkent@chromium.org

Change-Id: I3518cb22f1b19fb4a2942c46fbcd9e7326d20863
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 956832, 1115515
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352430Reviewed-by: default avatarStephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797234}
parent 55aed832
Hide whitespace changes
Inline Side-by-side
Showing with 19 additions and 113 deletions
components/printing/renderer/print_render_frame_helper.cc
View file @ 87248e6a
......@@ -47,7 +47,6 @@
#include "third_party/blink/public/platform/platform.h"
#include "third_party/blink/public/platform/web_data.h"
#include "third_party/blink/public/platform/web_double_size.h"
#include "third_party/blink/public/platform/web_scoped_page_pauser.h"
#include "third_party/blink/public/platform/web_size.h"
#include "third_party/blink/public/platform/web_url.h"
#include "third_party/blink/public/platform/web_url_request.h"
......@@ -1113,11 +1112,6 @@ void PrintRenderFrameHelper::DidStartNavigation(
const GURL& url,
base::Optional<blink::WebNavigationType> navigation_type) {
is_loading_ = true;
// If the renderer navigates while paused, unpause to let the navigation
// proceed.
if (print_preview_context_.IsPaused())
auto pauser = print_preview_context_.TakePauser();
}
void PrintRenderFrameHelper::DidFailProvisionalLoad() {
......@@ -1152,12 +1146,7 @@ void PrintRenderFrameHelper::ScriptedPrint(bool user_initiated) {
web_frame->DispatchBeforePrintEvent();
if (!weak_this)
return;
// Pause between onbeforeprint and onafterprint events.
// https://html.spec.whatwg.org/C/#printing-steps
Print(web_frame, blink::WebNode(), PrintRequestType::kScripted,
blink::WebScopedPagePauser::Create());
Print(web_frame, blink::WebNode(), PrintRequestType::kScripted);
if (weak_this)
web_frame->DispatchAfterPrintEvent();
}
......@@ -1192,12 +1181,7 @@ void PrintRenderFrameHelper::PrintRequestedPages() {
// If we are printing a PDF extension frame, find the plugin node and print
// that instead.
auto plugin = delegate_->GetPdfElement(frame);
// Pause between onbeforeprint and onafterprint events.
// https://html.spec.whatwg.org/C/#printing-steps
Print(frame, plugin, PrintRequestType::kRegular,
blink::WebScopedPagePauser::Create());
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
......@@ -1213,13 +1197,10 @@ void PrintRenderFrameHelper::PrintForSystemDialog() {
NOTREACHED();
return;
}
// Hand over control, including Print Preview's WebScopedPagePauser, to the
// system print dialog.
Print(frame, print_preview_context_.source_node(), PrintRequestType::kRegular,
print_preview_context_.TakePauser());
Print(frame, print_preview_context_.source_node(),
PrintRequestType::kRegular);
if (!render_frame_gone_)
print_preview_context_.DispatchAfterPrintEvent();
frame->DispatchAfterPrintEvent();
// WARNING: |this| may be gone at this point. Do not do any more work here and
// just return.
}
......@@ -1314,7 +1295,7 @@ void PrintRenderFrameHelper::PrintPreview(base::Value settings) {
void PrintRenderFrameHelper::OnPrintPreviewDialogClosed() {
ScopedIPC scoped_ipc(weak_ptr_factory_.GetWeakPtr());
print_preview_context_.DispatchAfterPrintEvent();
print_preview_context_.source_frame()->DispatchAfterPrintEvent();
}
#endif // BUILDFLAG(ENABLE_PRINT_PREVIEW)
......@@ -1364,9 +1345,6 @@ void PrintRenderFrameHelper::PrintFrameContent(
// try to handle pdf plugin element until that bug is fixed.
{
TRACE_EVENT0("print", "PrintRenderFrameHelper::PrintFrameContent");
// Pause between onbeforeprint and onafterprint events.
// https://html.spec.whatwg.org/C/#printing-steps
auto page_pauser = blink::WebScopedPagePauser::Create();
if (frame->PrintBegin(web_print_params,
/*constrain_to_node=*/blink::WebElement())) {
frame->PrintPage(0, canvas);
......@@ -1464,12 +1442,6 @@ void PrintRenderFrameHelper::PrepareFrameForPreviewDocument() {
prep_frame_view_ = std::make_unique<PrepareFrameAndViewForPrint>(
print_params, print_preview_context_.source_frame(),
print_preview_context_.source_node(), ignore_css_margins_);
// The renderer needs to load a new page to print the selection. Must unpause
// to do that.
if (print_params.selection_only)
auto pauser_to_destroy = print_preview_context_.TakePauser();
prep_frame_view_->CopySelectionIfNeeded(
render_frame()->GetWebkitPreferences(),
base::BindOnce(&PrintRenderFrameHelper::OnFramePreparedForPreviewDocument,
......@@ -1481,16 +1453,7 @@ void PrintRenderFrameHelper::OnFramePreparedForPreviewDocument() {
PrepareFrameForPreviewDocument();
return;
}
CreatePreviewDocumentResult result = CreatePreviewDocument();
// Now that the renderer has finished generating the print preview for a
// selection, pause again. The system print dialog path in Print() does not
// need to do this, since that dialog is done, whereas the Print Preview
// dialog will continue to generate more previews.
if (print_pages_params_->params.selection_only)
print_preview_context_.Pause();
if (result != CREATE_IN_PROGRESS)
DidFinishPrinting(result == CREATE_SUCCESS ? OK : FAIL_PREVIEW);
}
......@@ -1816,7 +1779,7 @@ void PrintRenderFrameHelper::PrintNode(const blink::WebNode& node) {
return;
Print(duplicate_node.GetDocument().GetFrame(), duplicate_node,
PrintRequestType::kRegular, blink::WebScopedPagePauser::Create());
PrintRequestType::kRegular);
// Check if |this| is still valid.
if (!weak_this)
return;
......@@ -1829,11 +1792,9 @@ void PrintRenderFrameHelper::PrintNode(const blink::WebNode& node) {
print_node_in_progress_ = false;
}
void PrintRenderFrameHelper::Print(
blink::WebLocalFrame* frame,
const blink::WebNode& node,
PrintRequestType print_request_type,
std::unique_ptr<blink::WebScopedPagePauser> pauser) {
void PrintRenderFrameHelper::Print(blink::WebLocalFrame* frame,
const blink::WebNode& node,
PrintRequestType print_request_type) {
// If still not finished with earlier print request simply ignore.
if (prep_frame_view_)
return;
......@@ -1879,11 +1840,6 @@ void PrintRenderFrameHelper::Print(
}
}
// The renderer needs to load a new page to print the selection. Must unpause
// to do that.
if (print_pages_params_->params.selection_only)
pauser.reset();
// Render Pages for printing.
if (!RenderPagesForPrint(frame_ref.GetFrame(), node)) {
LOG(ERROR) << "RenderPagesForPrint failed";
......@@ -2375,10 +2331,9 @@ void PrintRenderFrameHelper::ShowScriptedPrintPreview() {
void PrintRenderFrameHelper::RequestPrintPreview(PrintPreviewRequestType type) {
auto weak_this = weak_ptr_factory_.GetWeakPtr();
print_preview_context_.DispatchBeforePrintEvent(weak_this);
print_preview_context_.source_frame()->DispatchBeforePrintEvent();
if (!weak_this)
return;
const bool is_from_arc = print_preview_context_.IsForArc();
const bool is_modifiable = print_preview_context_.IsModifiable();
const bool is_pdf = print_preview_context_.IsPdf();
......@@ -2536,12 +2491,7 @@ void PrintRenderFrameHelper::OnPreviewDisconnect() {
PrintRenderFrameHelper::PrintPreviewContext::PrintPreviewContext() = default;
PrintRenderFrameHelper::PrintPreviewContext::~PrintPreviewContext() {
// Make sure |pauser_| is null. If |pauser_| still exists, it will try to
// unpause during teardown, and that is too late.
// DispatchAfterPrintEvent() or TakePauser() should have destroyed it already.
DCHECK(!pauser_);
}
PrintRenderFrameHelper::PrintPreviewContext::~PrintPreviewContext() = default;
void PrintRenderFrameHelper::PrintPreviewContext::InitWithFrame(
blink::WebLocalFrame* web_frame) {
......@@ -2564,38 +2514,6 @@ void PrintRenderFrameHelper::PrintPreviewContext::InitWithNode(
CalculatePluginAttributes();
}
bool PrintRenderFrameHelper::PrintPreviewContext::IsPaused() const {
return !!pauser_;
}
void PrintRenderFrameHelper::PrintPreviewContext::Pause() {
DCHECK(!pauser_);
pauser_ = blink::WebScopedPagePauser::Create();
}
std::unique_ptr<blink::WebScopedPagePauser>
PrintRenderFrameHelper::PrintPreviewContext::TakePauser() {
DCHECK(pauser_);
return std::move(pauser_);
}
void PrintRenderFrameHelper::PrintPreviewContext::DispatchBeforePrintEvent(
base::WeakPtr<PrintRenderFrameHelper> weak_this) {
source_frame()->DispatchBeforePrintEvent();
if (!weak_this)
return;
DCHECK(!pauser_);
pauser_ = blink::WebScopedPagePauser::Create();
}
void PrintRenderFrameHelper::PrintPreviewContext::DispatchAfterPrintEvent() {
// No DCHECK(pauser_), as |pauser_| may have been reset by TakePauser().
pauser_.reset();
source_frame()->DispatchAfterPrintEvent();
}
void PrintRenderFrameHelper::PrintPreviewContext::OnPrintPreview() {
DCHECK_EQ(INITIALIZED, state_);
ClearContext();
......
components/printing/renderer/print_render_frame_helper.h
View file @ 87248e6a
......@@ -50,9 +50,8 @@ class DictionaryValue;
namespace blink {
class WebLocalFrame;
class WebScopedPagePauser;
class WebView;
} // namespace blink
}
namespace content {
class AXTreeSnapshotter;
......@@ -292,8 +291,7 @@ class PrintRenderFrameHelper
// WARNING: |this| may be gone after this method returns.
void Print(blink::WebLocalFrame* frame,
const blink::WebNode& node,
PrintRequestType print_request_type,
std::unique_ptr<blink::WebScopedPagePauser> pauser);
PrintRequestType print_request_type);
// Notification when printing is done - signal tear-down/free resources.
void DidFinishPrinting(PrintingResult result);
......@@ -475,17 +473,6 @@ class PrintRenderFrameHelper
void InitWithFrame(blink::WebLocalFrame* web_frame);
void InitWithNode(const blink::WebNode& web_node);
// Manual control of pausing/unpausing for special situations.
bool IsPaused() const;
void Pause();
std::unique_ptr<blink::WebScopedPagePauser> TakePauser();
// Dispatchs onbeforeprint/onafterprint events. Use these instead of calling
// the WebLocalFrame version on source_frame().
void DispatchBeforePrintEvent(
base::WeakPtr<PrintRenderFrameHelper> weak_this);
void DispatchAfterPrintEvent();
// Does bookkeeping at the beginning of print preview.
void OnPrintPreview();
......@@ -571,10 +558,6 @@ class PrintRenderFrameHelper
std::unique_ptr<PrepareFrameAndViewForPrint> prep_frame_view_;
// Manages when to pause between onbeforeprint and onafterprint events.
// https://html.spec.whatwg.org/C/#printing-steps
std::unique_ptr<blink::WebScopedPagePauser> pauser_;
// The typefaces encountered in the content during document serialization.
ContentProxySet typeface_content_info_;
......
third_party/blink/renderer/core/page/chrome_client.cc
View file @ 87248e6a
......@@ -292,6 +292,11 @@ bool ChromeClient::Print(LocalFrame* frame) {
return false;
}
// Suspend pages in case the client method runs a new event loop that would
// otherwise cause the load to continue while we're in the middle of
// executing JavaScript.
ScopedPagePauser pauser;
PrintDelegate(frame);
return true;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment