Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1169263003:
Change return type of PrintPage (Closed)
|
Created:
5 years, 6 months ago by Habib Virji Modified:
5 years, 6 months ago CC:
chromium-reviews, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, jam, darin-cc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChange return type of PrintPage
Patchset #3 to land chromium side changes for changing return type of the PrintPage.
BUG=496765
R=piman@chromium.org, raymes
Committed: https://crrev.com/910c326be112c84f9df0159da7cdcde38a0fbc75
Cr-Commit-Position: refs/heads/master@{#334573}
Patch Set 1 #
Total comments: 4
Patch Set 2 : bool printPage removal #
Messages
Total messages: 21 (8 generated)
The CQ bit was checked by habib.virji@samsung.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1169263003/1
This change will involve a total set of five patches. This is patch #3 from the below list. 1. Chromium side with two functions, one with old bool return type and other with three parameter but with void return type. 2. Blink side with old bool return function plus a function with void return type and an additional third parameter. 3. Chromium side with two functions with different parameters, return type is not changed. One function that removes third parameter added in first CL and other function keeping the third parameter. 4. Blink side remove function with three parameters. 5. Chromium side remove function with three parameters.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... File content/renderer/pepper/pepper_webplugin_impl.cc (right): https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... content/renderer/pepper/pepper_webplugin_impl.cc:282: nit: please remove redundant empty line
https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... File content/renderer/pepper/pepper_webplugin_impl.cc (right): https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... content/renderer/pepper/pepper_webplugin_impl.cc:279: return instance_->PrintPage(page_number, canvas); This "return" can be removed right?
https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... File content/renderer/pepper/pepper_webplugin_impl.cc (right): https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... content/renderer/pepper/pepper_webplugin_impl.cc:279: return instance_->PrintPage(page_number, canvas); That's true removed. Also below printEnd which does same has also been removed. https://codereview.chromium.org/1169263003/diff/1/content/renderer/pepper/pep... content/renderer/pepper/pepper_webplugin_impl.cc:282: Thanks. Done.
The CQ bit was checked by habib.virji@samsung.com to run a CQ dry run
The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org Link to the patchset: https://codereview.chromium.org/1169263003/#ps20001 (title: "bool printPage removal")
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1169263003/20001
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 habib.virji@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1169263003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by habib.virji@samsung.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1169263003/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/910c326be112c84f9df0159da7cdcde38a0fbc75 Cr-Commit-Position: refs/heads/master@{#334573} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
