Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2973743002:
Add plugin stub for print preview tests (Closed)
DescriptionAdd plugin stub for print preview tests
Add a plugin stub to trigger the load callback needed for print preview
tests.
BUG=717296
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
Review-Url: https://codereview.chromium.org/2973743002
Cr-Commit-Position: refs/heads/master@{#484964}
Committed: https://chromium.googlesource.com/chromium/src/+/f90653e7e0200081afc19c5ebae8c58fd40bce14
Patch Set 1 : Fix #Patch Set 2 : Fix comments #
Total comments: 10
Patch Set 3 : Address comments #
Total comments: 2
Patch Set 4 : Update annotation #
Dependent Patchsets: Messages
Total messages: 31 (24 generated)
Description was changed from ========== Add plugin stub for print preview tests Add a plugin stub to trigger the load callback needed for print preview tests. BUG=717296 ========== to ========== Add plugin stub for print preview tests Add a plugin stub to trigger the load callback needed for print preview tests. BUG=717296 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
Description was changed from ========== Add plugin stub for print preview tests Add a plugin stub to trigger the load callback needed for print preview tests. BUG=717296 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Add plugin stub for print preview tests Add a plugin stub to trigger the load callback needed for print preview tests. BUG=717296 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ==========
rbpotter@chromium.org changed reviewers: + dpapad@chromium.org
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Patchset #1 (id:1) has been deleted
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... File chrome/test/data/webui/print_preview/plugin_stub.js (right): https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:8: * @param {print_preview.PreviewArea} The PreviewArea that owns this plugin. !print_preview.PreviewArea https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:11: function PDFPluginStub(area) { Can we use the new ES6 class syntax, which we recently allowed in the styleguide (see [1])? See example CL at https://codereview.chromium.org/2960293002. While we are not aggressively converting all existing code to use it, it makes sense to use it in new code. [1] https://chromium.googlesource.com/chromium/src/+/286a6b8e56a7bd0b7ac58d8b9ba9... https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:12: /** @private {Object} The callback to run when the plugin has loaded. */ ?Function https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:15: /** @private {EventTracker} The plugin stub's event tracker. */ !EventTracker https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:42: * @param {Array<int>} pages The pages to preview. !Array<number>
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... File chrome/test/data/webui/print_preview/plugin_stub.js (right): https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:8: * @param {print_preview.PreviewArea} The PreviewArea that owns this plugin. On 2017/07/06 00:24:17, dpapad wrote: > !print_preview.PreviewArea Done. https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:11: function PDFPluginStub(area) { On 2017/07/06 00:24:17, dpapad wrote: > Can we use the new ES6 class syntax, which we recently allowed in the styleguide > (see [1])? See example CL at https://codereview.chromium.org/2960293002. > > While we are not aggressively converting all existing code to use it, it makes > sense to use it in new code. > > [1] > https://chromium.googlesource.com/chromium/src/+/286a6b8e56a7bd0b7ac58d8b9ba9... Done. https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:12: /** @private {Object} The callback to run when the plugin has loaded. */ On 2017/07/06 00:24:17, dpapad wrote: > ?Function Done. https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:15: /** @private {EventTracker} The plugin stub's event tracker. */ On 2017/07/06 00:24:17, dpapad wrote: > !EventTracker Done. https://codereview.chromium.org/2973743002/diff/40001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:42: * @param {Array<int>} pages The pages to preview. On 2017/07/06 00:24:17, dpapad wrote: > !Array<number> Done.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2973743002/diff/60001/chrome/test/data/webui/... File chrome/test/data/webui/print_preview/plugin_stub.js (right): https://codereview.chromium.org/2973743002/diff/60001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:32: * @param {?Function} callback The callback to run when the plugin has Should we ever be calling setLoadCallback(null)? If not let's update this to be !Function
The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run
https://codereview.chromium.org/2973743002/diff/60001/chrome/test/data/webui/... File chrome/test/data/webui/print_preview/plugin_stub.js (right): https://codereview.chromium.org/2973743002/diff/60001/chrome/test/data/webui/... chrome/test/data/webui/print_preview/plugin_stub.js:32: * @param {?Function} callback The callback to run when the plugin has On 2017/07/06 16:42:24, dpapad wrote: > Should we ever be calling setLoadCallback(null)? If not let's update this to be > !Function Done. I was thinking maybe we would want to clear it sometimes, but currently there is no use for this. Can always change it back later if this needs to happen for some reason.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
The CQ bit was checked by rbpotter@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from dpapad@chromium.org Link to the patchset: https://codereview.chromium.org/2973743002/#ps80001 (title: "Update annotation")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1499447664934980,
"parent_rev": "431258198d910ec58ffa149817c680a8ab5ddc61", "commit_rev":
"f90653e7e0200081afc19c5ebae8c58fd40bce14"}
Message was sent while issue was closed.
Description was changed from ========== Add plugin stub for print preview tests Add a plugin stub to trigger the load callback needed for print preview tests. BUG=717296 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation ========== to ========== Add plugin stub for print preview tests Add a plugin stub to trigger the load callback needed for print preview tests. BUG=717296 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2973743002 Cr-Commit-Position: refs/heads/master@{#484964} Committed: https://chromium.googlesource.com/chromium/src/+/f90653e7e0200081afc19c5ebae8... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:80001) as https://chromium.googlesource.com/chromium/src/+/f90653e7e0200081afc19c5ebae8... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
