printPage to return void instead of bool

printPage to return void instead of bool

Changes return type of the printPage. Since it requires changes on the chromium side too, an additional parameter has been added.

R=pdr
BUG=496765
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=196955

[Habib Virji, 2015-06-08 17:16:29]

This change will involve a total set of five patches. This is patch 2 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.

[pdr., 2015-06-08 22:30:32]

We don't need 5 patches to land this, I think it can be done in 3.

Patch 1 in blink:
  1) Add virtual void printPage(WebCanvas* canvas, int pageNumber) { }
  2) Change callsite to call both printPage functions.

Patch 2 in content:
  1) Implement the new printPage, remove the old one.

Patch 3 in blink:
  1) Remove the old printPage.

[Habib Virji, 2015-06-09 09:05:37]

Yes that could have been possible had it been not change of return type. 

It only allows to function overload when the parameter differs not the return
type. Two functions with same return type, void printPage and bool printPage,
will not compile as vtable cannot differentiate it. That's the reason I was
adding extra parameter to bring it to above stage you mentioned.

[pdr., 2015-06-09 20:28:32]

On 2015/06/09 at 09:05:37, habib.virji wrote:
> Yes that could have been possible had it been not change of return type. 
> 
> It only allows to function overload when the parameter differs not the return
type. Two functions with same return type, void printPage and bool printPage,
will not compile as vtable cannot differentiate it. That's the reason I was
adding extra parameter to bring it to above stage you mentioned.

Ah, great point. I'm not an owner of content/ but I've pinged some folks to
review patch #1. I think this plan sounds good.

[Habib Virji, 2015-06-10 15:50:42]

Thanks. content/ change has landed.

[pdr., 2015-06-10 16:59:14]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-06-10 16:59:14]

lgtm

[commit-bot: I haz the power, 2015-06-10 16:59:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1165103004/1

[commit-bot: I haz the power, 2015-06-10 17:09:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-10 17:09:46]

Try jobs failed on following builders:
  linux_chromium_gn_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_chromium_gn_rel/bu...)
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/58416)

[Habib Virji, 2015-06-11 10:20:15]

Patchset #3 (id:40001) has been deleted

[Habib Virji, 2015-06-11 10:20:23]

Patchset #2 (id:20001) has been deleted

[Habib Virji, 2015-06-11 10:24:57]

Patchset #2 (id:60001) has been deleted

[Habib Virji, 2015-06-11 13:02:32]

The CQ bit was checked by habib.virji@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2015-06-11 13:02:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1165103004/90001

[commit-bot: I haz the power, 2015-06-11 13:58:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-06-11 13:58:04]

Dry run: This issue passed the CQ dry run.

[Habib Virji, 2015-06-11 14:00:52]

The CQ bit was checked by habib.virji@samsung.com

[Habib Virji, 2015-06-11 14:00:53]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1165103004/#ps110001
(title: "Rebaseline fixes")

[commit-bot: I haz the power, 2015-06-11 14:01:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1165103004/110001

[Habib Virji, 2015-06-11 14:32:53]

@pdr: There were fix updates about your l-g-t-m as there was a compilation issue
and rebaseline needed. The main code for adding additional function remains.

[commit-bot: I haz the power, 2015-06-11 15:18:33]

Committed patchset #4 (id:110001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196955