New issue
Advanced search Search tips

Issue 746768 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

Delete print preview's generate draft feature

Project Member Reported by thestig@chromium.org, Jul 20 2017

Issue description

Print Preview originally had an optimization that avoided re-rendering the preview for cases where the settings haven't changed much. e.g. When the only difference between two preview requests is the page range going from all pages to page 2, Print Preview can reuse the output of the previous preview request.

Over time, this optimization bit rotted and nobody noticed until recently. Trying to revive it hit snags like  bug 731600  and  bug 740830 , so the feature is being disable for now.

We should either try to revive it again, or decide it it not worth it and delete it altogether.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 20 2017

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

commit 34e22410141c1c4d3c7efe5ee821b09d837bf995
Author: Lei Zhang <thestig@chromium.org>
Date: Thu Jul 20 04:05:10 2017

Print Preview: Disable Generate Draft feature.

The attempt to restore this feature in r469749 did not quite do the job
and has several issues.

BUG= 731600 , 740830 , 746768 

Change-Id: I417611e32e6d3e1bb29c96f6e5de6eee39abc846
Reviewed-on: https://chromium-review.googlesource.com/577374
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488129}
[modify] https://crrev.com/34e22410141c1c4d3c7efe5ee821b09d837bf995/chrome/browser/resources/print_preview/preview_generator.js
[modify] https://crrev.com/34e22410141c1c4d3c7efe5ee821b09d837bf995/chrome/test/data/webui/print_preview/print_preview_ui_browsertest.js

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 21 2017

Labels: merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d02baa474b267c2b53ac85a6faa3279310163f30

commit d02baa474b267c2b53ac85a6faa3279310163f30
Author: Lei Zhang <thestig@chromium.org>
Date: Fri Jul 21 22:40:18 2017

M60: Print Preview: Disable Generate Draft feature.

The attempt to restore this feature in r469749 did not quite do the job
and has several issues.

This is not cherry-picked from the master branch because the related
test got rewritten.

BUG= 731600 , 740830 , 746768 

Cr-Original-Original-Commit-Position: refs/heads/master@{#488129}
Reviewed-on: https://chromium-review.googlesource.com/577374
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I8c7ec7bb50c390ad6aef603885f41cb46f8d05e2
Reviewed-on: https://chromium-review.googlesource.com/581676
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#662}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/d02baa474b267c2b53ac85a6faa3279310163f30/chrome/browser/resources/print_preview/preview_generator.js
[modify] https://crrev.com/d02baa474b267c2b53ac85a6faa3279310163f30/chrome/test/data/webui/print_preview/print_preview.js

Project Member

Comment 3 by bugdroid1@chromium.org, Feb 6 2018

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

commit d925a938226f7ff7438a085f5f8a1b5cc99ba297
Author: Lei Zhang <thestig@chromium.org>
Date: Tue Feb 06 21:49:31 2018

Remove Print Preview's generate draft feature in the renderer.

The feature has lived on for too long in a broken state, and just makes
the code more complicated than necessary. Remove the rendering code
that deals with generating draft pages. There still exists browser side
code to be removed later.

This CL makes a PrintPreviewFailureType enum value obsolete. While
marking it as deprecated, also fix another issue with
PrintPreviewFailureType where the wrong enum got marked as deprecated.

BUG= 746768 

Change-Id: I5a61770d034ff33add6f4070984e46fa0a02b221
Reviewed-on: https://chromium-review.googlesource.com/899855
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Mark Pearson <mpearson@chromium.org>
Reviewed-by: Wei Li <weili@chromium.org>
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534806}
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/chrome/browser/printing/print_preview_message_handler.cc
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/components/printing/common/print_messages.h
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/components/printing/renderer/print_render_frame_helper.cc
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/components/printing/renderer/print_render_frame_helper.h
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/components/printing/renderer/print_render_frame_helper_mac.mm
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/components/printing/test/print_render_frame_helper_browsertest.cc
[modify] https://crrev.com/d925a938226f7ff7438a085f5f8a1b5cc99ba297/tools/metrics/histograms/enums.xml

Summary: Delete print preview's generate draft feature (was: Revive print preview's generate draft feature)
It turns out this actually started a long time ago with r227717 and r236642. The original reason was that the generate draft feature only helped in 0.5% of the preview requests.
Project Member

Comment 6 by bugdroid1@chromium.org, May 4 2018

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

commit bab79f05863f0a2363ccd20e1f589ffee7637400
Author: Lei Zhang <thestig@chromium.org>
Date: Fri May 04 19:34:45 2018

Printing: Finish removing the generate draft feature.

BUG= 746768 

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I70bf50ac9a9b4708af4bfac0e975a727c9a82edb
Reviewed-on: https://chromium-review.googlesource.com/1043649
Reviewed-by: Rebekah Potter <rbpotter@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556151}
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/printing/print_preview_data_service.cc
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/printing/print_preview_data_service.h
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/resources/print_preview/new/preview_area.js
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/resources/print_preview/preview_generator.js
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/ui/webui/print_preview/print_preview_handler_unittest.cc
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/browser/ui/webui/print_preview/print_preview_ui.h
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/test/data/webui/print_preview/print_preview_tests.js
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/chrome/test/data/webui/print_preview/print_preview_ui_browsertest.js
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/components/printing/test/print_render_frame_helper_browsertest.cc
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/printing/print_job_constants.cc
[modify] https://crrev.com/bab79f05863f0a2363ccd20e1f589ffee7637400/printing/print_job_constants.h

Status: Fixed (was: Assigned)

Sign in to add a comment