Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(279)

Issue 2938073003: Change getAccessToken and getExtensionPrinterAccess to sendWithPromise (Closed)

Created:
3 years, 6 months ago by rbpotter
Modified:
3 years, 6 months ago
Reviewers:
dpapad
CC:
arv+watch_chromium.org, chromium-reviews, Vitaly Buka (NO REVIEWS)
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

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} Committed: https://chromium.googlesource.com/chromium/src/+/699c1708fea286497554a012970ddb99a4123c91

Patch Set 1 #

Patch Set 2 : Cleanup #

Total comments: 6

Patch Set 3 : Address comments #

Total comments: 10

Patch Set 4 : Address comments #

Total comments: 8

Patch Set 5 : Comments #

Patch Set 6 : Remove map and extra dest type #

Total comments: 6

Patch Set 7 : Address comments, remove extra code #

Total comments: 4

Patch Set 8 : Remove queue #

Total comments: 2

Patch Set 9 : Fix annotation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+143 lines, -187 lines) Patch
M chrome/browser/resources/print_preview/cloud_print_interface.js View 1 2 3 4 5 6 7 8 6 chunks +32 lines, -50 lines 0 comments Download
M chrome/browser/resources/print_preview/data/destination.js View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M chrome/browser/resources/print_preview/data/destination_store.js View 1 2 3 4 5 6 5 chunks +54 lines, -46 lines 0 comments Download
M chrome/browser/resources/print_preview/data/local_parsers.js View 1 2 3 4 5 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/resources/print_preview/native_layer.js View 1 2 3 4 5 8 chunks +18 lines, -65 lines 0 comments Download
M chrome/browser/ui/webui/print_preview/print_preview_handler.h View 2 chunks +6 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/print_preview/print_preview_handler.cc View 1 2 3 7 chunks +31 lines, -21 lines 0 comments Download

Messages

Total messages: 61 (46 generated)
rbpotter
3 years, 6 months ago (2017-06-15 20:57:36 UTC) #12
dpapad
https://codereview.chromium.org/2938073003/diff/20001/chrome/browser/resources/print_preview/data/destination_store.js File chrome/browser/resources/print_preview/data/destination_store.js (right): https://codereview.chromium.org/2938073003/diff/20001/chrome/browser/resources/print_preview/data/destination_store.js#newcode1018 chrome/browser/resources/print_preview/data/destination_store.js:1018: this.handleProvisionalDestinationResolved_.bind(this, id), handleProvisionalDestinationResolved_and handleProvisionalDestinationRejected_ are only used here. Can ...
3 years, 6 months ago (2017-06-15 22:20:13 UTC) #15
rbpotter
https://codereview.chromium.org/2938073003/diff/20001/chrome/browser/resources/print_preview/data/destination_store.js File chrome/browser/resources/print_preview/data/destination_store.js (right): https://codereview.chromium.org/2938073003/diff/20001/chrome/browser/resources/print_preview/data/destination_store.js#newcode1018 chrome/browser/resources/print_preview/data/destination_store.js:1018: this.handleProvisionalDestinationResolved_.bind(this, id), On 2017/06/15 22:20:13, dpapad wrote: > handleProvisionalDestinationResolved_and ...
3 years, 6 months ago (2017-06-16 02:19:36 UTC) #19
dpapad
https://codereview.chromium.org/2938073003/diff/60001/chrome/browser/resources/print_preview/data/destination_store.js File chrome/browser/resources/print_preview/data/destination_store.js (right): https://codereview.chromium.org/2938073003/diff/60001/chrome/browser/resources/print_preview/data/destination_store.js#newcode1019 chrome/browser/resources/print_preview/data/destination_store.js:1019: * Removes the destination from the store and replaces ...
3 years, 6 months ago (2017-06-16 03:01:00 UTC) #20
rbpotter
https://codereview.chromium.org/2938073003/diff/60001/chrome/browser/resources/print_preview/data/destination_store.js File chrome/browser/resources/print_preview/data/destination_store.js (right): https://codereview.chromium.org/2938073003/diff/60001/chrome/browser/resources/print_preview/data/destination_store.js#newcode1019 chrome/browser/resources/print_preview/data/destination_store.js:1019: * Removes the destination from the store and replaces ...
3 years, 6 months ago (2017-06-16 04:32:07 UTC) #25
dpapad
https://codereview.chromium.org/2938073003/diff/80001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/80001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode109 chrome/browser/resources/print_preview/cloud_print_interface.js:109: this.accessTokenRequestMap_ = new Map(); Let's wait for @vitalybuka's response ...
3 years, 6 months ago (2017-06-16 22:21:30 UTC) #28
rbpotter
https://codereview.chromium.org/2938073003/diff/80001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/80001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode109 chrome/browser/resources/print_preview/cloud_print_interface.js:109: this.accessTokenRequestMap_ = new Map(); On 2017/06/16 22:21:29, dpapad wrote: ...
3 years, 6 months ago (2017-06-19 22:19:30 UTC) #35
dpapad
https://codereview.chromium.org/2938073003/diff/120001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/120001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode394 chrome/browser/resources/print_preview/cloud_print_interface.js:394: this.accessTokenRequestPromise_ = this.accessTokenRequestPromise_ || A bit confused by this ...
3 years, 6 months ago (2017-06-20 01:36:21 UTC) #38
rbpotter
https://codereview.chromium.org/2938073003/diff/120001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/120001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode394 chrome/browser/resources/print_preview/cloud_print_interface.js:394: this.accessTokenRequestPromise_ = this.accessTokenRequestPromise_ || On 2017/06/20 01:36:21, dpapad wrote: ...
3 years, 6 months ago (2017-06-20 02:47:54 UTC) #39
dpapad
https://codereview.chromium.org/2938073003/diff/140001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/140001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode391 chrome/browser/resources/print_preview/cloud_print_interface.js:391: return this.sendRequest_(request); This method is not declaring any @return. ...
3 years, 6 months ago (2017-06-20 16:09:09 UTC) #44
rbpotter
https://codereview.chromium.org/2938073003/diff/140001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/140001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode391 chrome/browser/resources/print_preview/cloud_print_interface.js:391: return this.sendRequest_(request); On 2017/06/20 16:09:09, dpapad wrote: > This ...
3 years, 6 months ago (2017-06-21 03:52:28 UTC) #49
dpapad
LGTM https://codereview.chromium.org/2938073003/diff/160001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/160001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode100 chrome/browser/resources/print_preview/cloud_print_interface.js:100: * @private {?Promise} I believe this is now ...
3 years, 6 months ago (2017-06-21 18:18:42 UTC) #50
rbpotter
https://codereview.chromium.org/2938073003/diff/160001/chrome/browser/resources/print_preview/cloud_print_interface.js File chrome/browser/resources/print_preview/cloud_print_interface.js (right): https://codereview.chromium.org/2938073003/diff/160001/chrome/browser/resources/print_preview/cloud_print_interface.js#newcode100 chrome/browser/resources/print_preview/cloud_print_interface.js:100: * @private {?Promise} On 2017/06/21 18:18:41, dpapad wrote: > ...
3 years, 6 months ago (2017-06-21 19:14:30 UTC) #52
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2938073003/180001
3 years, 6 months ago (2017-06-22 04:50:03 UTC) #58
commit-bot: I haz the power
3 years, 6 months ago (2017-06-22 05:35:33 UTC) #61
Message was sent while issue was closed.
Committed patchset #9 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/699c1708fea286497554a012970d...

Powered by Google App Engine
This is Rietveld 408576698