New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 717296 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 549759



Sign in to add a comment

Print preview: Modernize C++/JS communication

Project Member Reported by dpa...@chromium.org, May 1 2017

Issue description

Print preview's C++/JS communication is using older patterns in WebUI, specifically:

1) Uses chrome.send() everywhere, instead of using cr.sendWithPromise() where
   possible. cr.sendWithPromise() makes 1:1 (request/response) communication much easier.
2) Exposing global Javascript methods [1] and calling them by name from C++ with
   CallJavascriptFunctionUnsafe [2], instead of using WebUI events (and cr.webUIListenerCallback)

The code can be simplified quite a bit and become more test friendly by following the pattern
described at [3].

Modernizing PP's C++/JS communication will make the code cleaner, will allow adding more robust tests
(with TestBrowserProxy concept), which eventually will allow the PP codebase to be easier to deal with
when it comes to a large scale UI refresh (Material Design / Harmony)

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/print_preview/native_layer.js?l=39-81
[2] https://cs.chromium.org/chromium/src/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[3] https://docs.google.com/document/d/1c20VYdwpUPyBRQeAS0CMr6ahwWnb0s26gByomOwqDjk/edit#heading=h.5co3z5yokc1c
 
Cc: thestig@chromium.org
Status: Available (was: Untriaged)
Project Member

Comment 3 by bugdroid1@chromium.org, May 22 2017

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

commit 4adaebf29293acf54c54ac2910f0a3bb2f32683f
Author: rbpotter <rbpotter@chromium.org>
Date: Mon May 22 22:59:41 2017

Print Preview: Use cr.sendWithPromise for getInitialSettings

Start replacing some chrome.send calls with cr.sendWithPromise in print
preview by using cr.sendWithPromise for the getInitialSettings message.
To avoid breaking tests during transition, also:
- Change NativeLayerStub in PrintPreviewWebUITests to inherit from
settings.TestBrowserProxy.
- Change NativeLayer so that it no longer inherits from cr.EventTarget.
- Add an EventTarget member to NativeLayer for dispatching and
receiving the print preview messages that have not yet been migrated
to use cr.sendWithPromise (will be removed when all messages have
been changed).

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2881213003
Cr-Commit-Position: refs/heads/master@{#473731}

[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/resources/print_preview/cloud_print_interface.js
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/resources/print_preview/preview_generator.js
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/test/data/webui/print_preview.js
[modify] https://crrev.com/4adaebf29293acf54c54ac2910f0a3bb2f32683f/chrome/test/data/webui/print_preview_destination_search_test.js

Project Member

Comment 4 by bugdroid1@chromium.org, May 22 2017

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

commit 7d5b724780d484c91a46232ec2fd6c51e0d15f37
Author: rbpotter <rbpotter@chromium.org>
Date: Mon May 22 23:15:09 2017

Print Preview: Merge NativeLayerStubs for tests

Move NativeLayerStub to a separate file and use the same stub for both
the PrintPreviewDestinationSearchTests and the PrintPreviewWebUITests.

Also move all print_preview related tests into a subfolder of
chrome/test/data/webui/ with the new native_layer_stub.js file.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2893003003
Cr-Commit-Position: refs/heads/master@{#473738}

[modify] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/test/data/webui/BUILD.gn
[add] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/test/data/webui/print_preview/native_layer_stub.js
[rename] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/test/data/webui/print_preview/print_preview.js
[rename] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/test/data/webui/print_preview/print_preview_destination_search_test.js
[rename] https://crrev.com/7d5b724780d484c91a46232ec2fd6c51e0d15f37/chrome/test/data/webui/print_preview/print_preview_hello_world_test.html

Project Member

Comment 5 by bugdroid1@chromium.org, Jun 2 2017

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

commit 10c0414243dada240a7fbac8853cb31d6ced5e86
Author: rbpotter <rbpotter@chromium.org>
Date: Fri Jun 02 00:39:01 2017

Print Preview: Change getPrinters to cr.sendWithPromise

Change one more native layer message to cr.sendWithPromise and adjust
tests to use browser proxy for this message instead of triggering an
event.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2919693002
Cr-Commit-Position: refs/heads/master@{#476503}

[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/browser/resources/print_preview/data/local_parsers.js
[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/10c0414243dada240a7fbac8853cb31d6ced5e86/chrome/test/data/webui/print_preview/print_preview_tests.js

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 8 2017

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

commit c0f846bdbfa4e685f05c48443923db9dfceaa35d
Author: rbpotter <rbpotter@chromium.org>
Date: Thu Jun 08 00:24:43 2017

Change getExtensionPrinters to cr.sendWithPromise

Use cr.sendWithPromise for getExtensionPrinters. Resolve the promise
only when all extension printers have been listed. Also add a Web UI
event to fire from C++ as individual lists of printers are ready.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2918243002
Cr-Commit-Position: refs/heads/master@{#477826}

[modify] https://crrev.com/c0f846bdbfa4e685f05c48443923db9dfceaa35d/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/c0f846bdbfa4e685f05c48443923db9dfceaa35d/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/c0f846bdbfa4e685f05c48443923db9dfceaa35d/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/c0f846bdbfa4e685f05c48443923db9dfceaa35d/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/c0f846bdbfa4e685f05c48443923db9dfceaa35d/chrome/test/data/webui/print_preview/native_layer_stub.js

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 8 2017

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

commit 269a344191b53f18ec5bfffbdd1ca6be2b3abc61
Author: rbpotter <rbpotter@chromium.org>
Date: Thu Jun 08 00:59:07 2017

Print Preview: Remove unused event

LOCAL_DESTINATIONS_SET is no longer used. Remove the event and
modify comment that references it.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2923363005
Cr-Commit-Position: refs/heads/master@{#477840}

[modify] https://crrev.com/269a344191b53f18ec5bfffbdd1ca6be2b3abc61/chrome/browser/resources/print_preview/native_layer.js

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 8 2017

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

commit 242c8082ae66fee2667469a64c3e4b91205fe75a
Author: rbpotter <rbpotter@chromium.org>
Date: Thu Jun 08 19:30:33 2017

Print Preview: Change getPrivetPrinters to cr.sendWithPromise

Change the getPrivetPrinters message to use cr.sendWithPromise. Also
move the timeout to C++ from Javascript, and resolve the promise on
timeout instead of sending a separate stop message from JS.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2920153004
Cr-Commit-Position: refs/heads/master@{#478053}

[modify] https://crrev.com/242c8082ae66fee2667469a64c3e4b91205fe75a/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/242c8082ae66fee2667469a64c3e4b91205fe75a/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/242c8082ae66fee2667469a64c3e4b91205fe75a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/242c8082ae66fee2667469a64c3e4b91205fe75a/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/242c8082ae66fee2667469a64c3e4b91205fe75a/chrome/test/data/webui/print_preview/native_layer_stub.js

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 14 2017

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

commit 03c1f4449fec6a5fe7406647023b0c8b6531444a
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Jun 14 20:32:30 2017

Print Preview: Change get*PrinterCapabilities to cr.sendWithPromise

Change all the get*PrinterCapabilities functions
(getPrinterCapabilities, getPrivetPrinterCapabilities,
getExtensionCapabilities) to cr.sendWithPromise. All share the same
function for failure (onGetCapabilitiesFail).

Change tests to work with the new system.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2931843003
Cr-Commit-Position: refs/heads/master@{#479489}

[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/resources/print_preview/data/local_parsers.js
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/ui/webui/print_preview/extension_printer_handler.cc
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/ui/webui/print_preview/extension_printer_handler.h
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/browser/ui/webui/print_preview/printer_handler.h
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/test/data/webui/print_preview/print_preview_destination_search_test.js
[modify] https://crrev.com/03c1f4449fec6a5fe7406647023b0c8b6531444a/chrome/test/data/webui/print_preview/print_preview_tests.js

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 15 2017

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

commit b6f4fde989a97dff96696a0c469a531c4b20557d
Author: dbeam <dbeam@chromium.org>
Date: Thu Jun 15 04:03:06 2017

Discourage new use of CallJavascriptFunctionUnsafe() via PRESUBMIT

R=dpranke@chromium.org
BUG= 717296 

Review-Url: https://codereview.chromium.org/2940893003
Cr-Commit-Position: refs/heads/master@{#479596}

[modify] https://crrev.com/b6f4fde989a97dff96696a0c469a531c4b20557d/PRESUBMIT.py

Project Member

Comment 11 by bugdroid1@chromium.org, Jun 15 2017

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

commit 30eeb64af965012272dc1ed7be0c0e6db5005871
Author: rbpotter <rbpotter@chromium.org>
Date: Thu Jun 15 19:23:22 2017

Print Preview: Remove global onPrivetPrintFailed

Remove global onPrivetPrintFailed method and replace with a WebUI
listener.

Also rename to onPrintFailed/print-failed since this can also be
called due to an extension printer failure.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2935303003
Cr-Commit-Position: refs/heads/master@{#479778}

[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/resources/print_preview/compiled_resources2.gyp
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/resources/print_preview/component.js
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/resources/print_preview/print_preview.html
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[add] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/ui/webui/resources/html/webui_listener_tracker.html
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/ui/webui/resources/js/compiled_resources2.gyp
[add] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/ui/webui/resources/js/webui_listener_tracker.js
[modify] https://crrev.com/30eeb64af965012272dc1ed7be0c0e6db5005871/ui/webui/resources/webui_resources.grd

Owner: rbpotter@chromium.org
Status: Started (was: Available)
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 22 2017

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

commit 699c1708fea286497554a012970ddb99a4123c91
Author: rbpotter <rbpotter@chromium.org>
Date: Thu Jun 22 05:35:17 2017

Change getAccessToken and getExtensionPrinterAccess to sendWithPromise

Change getAccessToken and getExtensionPrinterAccess to sendWithPromise
in print preview.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2938073003
Cr-Commit-Position: refs/heads/master@{#481451}

[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/resources/print_preview/cloud_print_interface.js
[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/resources/print_preview/data/destination.js
[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/resources/print_preview/data/local_parsers.js
[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/699c1708fea286497554a012970ddb99a4123c91/chrome/browser/ui/webui/print_preview/print_preview_handler.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 24 2017

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

commit d29fc0d18dc459eb543f50860d7d244694bebc5d
Author: rbpotter <rbpotter@chromium.org>
Date: Sat Jun 24 00:47:52 2017

Print Preview: Change print to cr.sendWithPromise

Change the print message in print preview to send with promise, and use
this to eliminate some global JS functions/CallJavascriptFunctionUnsafe:
- onFileSelectionCompleted
- onFileSelectionCancelled
- printToCloud

Also remove the print-failed webui event.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2948723002
Cr-Commit-Position: refs/heads/master@{#482095}

[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/browser/ui/webui/print_preview/print_preview_ui.h
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/d29fc0d18dc459eb543f50860d7d244694bebc5d/chrome/test/data/webui/print_preview/print_preview_tests.js

Project Member

Comment 15 by bugdroid1@chromium.org, Jun 28 2017

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

commit 437ea01a74fe57d58ef6e26c9579cebf60819b11
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Jun 28 02:41:47 2017

Print Preview: Make useCloudPrint a WebUI event

Remove the setUseCloudPrint global JS function and CallJSFunctionUnsafe
and replace with a use-cloud-print WebUI event.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2956033002
Cr-Commit-Position: refs/heads/master@{#482860}

[modify] https://crrev.com/437ea01a74fe57d58ef6e26c9579cebf60819b11/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/437ea01a74fe57d58ef6e26c9579cebf60819b11/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/437ea01a74fe57d58ef6e26c9579cebf60819b11/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/437ea01a74fe57d58ef6e26c9579cebf60819b11/chrome/test/data/webui/print_preview/print_preview_tests.js

Project Member

Comment 16 by bugdroid1@chromium.org, Jun 28 2017

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

commit 6c220aac9f218d0274a6bed442cce6211599811a
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Jun 28 16:52:32 2017

Print Preview: Update test to check parameters sent to print()

Update the native layer stub to pass on the parameters sent to print(),
and update the InvalidSettings test to sanity check some of these
parameters to verify print is being called correctly.

BUG= 717296 

Review-Url: https://codereview.chromium.org/2961773003
Cr-Commit-Position: refs/heads/master@{#483021}

[modify] https://crrev.com/6c220aac9f218d0274a6bed442cce6211599811a/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/6c220aac9f218d0274a6bed442cce6211599811a/chrome/test/data/webui/print_preview/print_preview_tests.js

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 5 2017

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

commit 579f9d6fc5f37f9c1a40894b8a811aad58d01d3b
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Jul 05 03:32:09 2017

Print Preview: change getPreview to cr.sendWithPromise

- Use cr.sendWithPromise for getPreview
- Remove invalidPrinterSettings, printPreviewFailed, and
  updatePrintPreview global JS functions
- Update some print preview tests to take advantage of browser proxy
  for getPreview.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2962983002
Cr-Commit-Position: refs/heads/master@{#484183}

[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/printing/print_preview_message_handler.cc
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/resources/print_preview/preview_generator.js
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/resources/print_preview/previewarea/preview_area.js
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/browser/ui/webui/print_preview/print_preview_ui.h
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/chrome/test/data/webui/print_preview/print_preview_tests.js
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/components/printing/renderer/print_web_view_helper.cc
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/components/printing/renderer/print_web_view_helper.h
[modify] https://crrev.com/579f9d6fc5f37f9c1a40894b8a811aad58d01d3b/components/printing/test/print_web_view_helper_browsertest.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 10 2017

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

commit c1665b3a3c4c1b59c69bca64af65aeb334a76311
Author: rbpotter <rbpotter@chromium.org>
Date: Mon Jul 10 22:41:51 2017

Print Preview: Finish removing global Javascript functions.

Remove remaining global javascript functions from the print preview
native layer and convert to sendWithPromise and web UI events.

BUG= 717296 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2969383003
Cr-Commit-Position: refs/heads/master@{#485430}

[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/resources/print_preview/preview_generator.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/resources/print_preview/previewarea/preview_area.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/ui/webui/print_preview/print_preview_handler.cc
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/ui/webui/print_preview/print_preview_handler.h
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/ui/webui/print_preview/print_preview_ui.cc
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/browser/ui/webui/print_preview/print_preview_ui.h
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/test/data/webui/print_preview/plugin_stub.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/test/data/webui/print_preview/print_preview_destination_search_test.js
[modify] https://crrev.com/c1665b3a3c4c1b59c69bca64af65aeb334a76311/chrome/test/data/webui/print_preview/print_preview_tests.js

Project Member

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

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

commit cae285de3cfae6a0b24adefcdbb56d5b67b46739
Author: rbpotter <rbpotter@chromium.org>
Date: Tue Jul 11 22:33:54 2017

Print Preview: Return or crash on failures

Ensure we always return after rejecting JS callback or CHECK if the
condition should always be met.

BUG= 717296 

Review-Url: https://codereview.chromium.org/2980603002
Cr-Commit-Position: refs/heads/master@{#485707}

[modify] https://crrev.com/cae285de3cfae6a0b24adefcdbb56d5b67b46739/chrome/browser/ui/webui/print_preview/print_preview_handler.cc

Blocking: 549759
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 19 2017

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

commit 1afc81225f4066c9c9408d36e51ec4195d7b6362
Author: rbpotter <rbpotter@chromium.org>
Date: Wed Jul 19 21:10:06 2017

Cleanup chrome.send in print preview

Move all chrome.send calls to the native layer
Rename native layer functions to match names of messages being sent.

Bug:  717296 
Change-Id: Ie69391d107183c4fd0e97339d4014ea89f1181e3
Reviewed-on: https://chromium-review.googlesource.com/575374
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Rebekah Potter <rbpotter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487979}
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/data/app_state.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/data/destination_store.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/metrics.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/native_layer.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/previewarea/preview_area.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/print_preview.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/browser/resources/print_preview/search/destination_search.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/test/data/webui/print_preview/native_layer_stub.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/test/data/webui/print_preview/print_preview_destination_search_test.js
[modify] https://crrev.com/1afc81225f4066c9c9408d36e51ec4195d7b6362/chrome/test/data/webui/print_preview/print_preview_tests.js

Status: Fixed (was: Started)
This is basically done. We can file individual bugs if we identify additional improvements. Thank you @rbpotter.

Sign in to add a comment