New issue
Advanced search Search tips

Issue 872983 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 14
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Incorrect renderer kill in PrintPreviewHandler when print preview fails.

Project Member Reported by thestig@chromium.org, Aug 9

Issue description

When I tried to simulate bug 872935 by modifying my local build to have PrintRenderFrameHelper::FinalizePrintReadyDocument() always fail, I noticed the tab gets killed when I try to print preview chrome://version. The tab killing happens inside PrintPreviewHandler::ShouldReceiveRendererMessage(), which is called by PrintPreviewHandler::SendPagePreviewReady().

The issue is due to print compositing, which can delay when print preview pages are ready on the browser side. If the renderer encounters a failure, it can notify the browser before compositing finishes. So PrintPreviewHandler::OnPrintPreviewFailed() gets called first for a given preview request. OnPrintPreviewFailed() handles it and forgets about the request's existence. Then compositing finishes for a page for the same request. But since PrintPreviewHandler no longer remembers the request, it interprets the situation as though the renderer sent a bad message.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ef800bf71664be777a73bd4a690b296fcd78ce7

commit 7ef800bf71664be777a73bd4a690b296fcd78ce7
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Aug 14 00:57:24 2018

Fix incorrect renderer kill in PrintPreviewHandler.

Make PrintPreviewHandler remember which preview requests failed. So when
out of order page ready events arrive after the failure due to
compositing, PrintPreviewHandler can figure out those events are safe to
ignore.

BUG= 872983 

Change-Id: I8b493a9b1efb50112495cf47ab25d1b01897d4c2
Reviewed-on: https://chromium-review.googlesource.com/1170228
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582772}
[modify] https://crrev.com/7ef800bf71664be777a73bd4a690b296fcd78ce7/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/7ef800bf71664be777a73bd4a690b296fcd78ce7/chrome/browser/ui/webui/print_preview/print_preview_handler.h

Status: Fixed (was: Assigned)

Sign in to add a comment