PdfMetafileSkia gives out a SkCanvas, not a SkBaseDevice.

PdfMetafileSkia gives out a SkCanvas, not a SkBaseDevice.

Motivation: hide SkPDFDevice and SkPDFDocument behind PdfMetafileSkia class.

pdf_metafile_skia.cc:
    Implement GetVectorCanvasForNewPage.
    Remove StartPageForVectorCanvas.
    Own the SkCanvas pointer (current_page_canvas_).
    Replace page_outstanding_ field with a method that checks to see
    if current_page_canvas_ is NULL.
    Implement SaveTo.

skia::VectorPlatformDeviceSkia
    remove class

print_web_view_helper.cc
    Remove calls to canvas->getTopDevice()->setDrawingArea(...); (SkDFDevice)
    This API is no longer effective.

print_web_view_helper_linux.cc,
print_web_view_helper_mac.mm,
print_web_view_helper_pdf_win.cc:
    call GetVectorCanvasForNewPage rather than StartPageForVectorCanvas

BUG=278148
Committed: https://crrev.com/5be808e0b7478b27304f5a29b47ba1af1de627d2
Cr-Commit-Position: refs/heads/master@{#303522}

[hal.canary, 2014-11-05 19:11:16]

halcanary@google.com changed reviewers:
+ reed@google.com, robertphillips@google.com, vitalybuka@chromium.org

[hal.canary, 2014-11-05 21:36:43]

Patchset #1 (id:1) has been deleted

[hal.canary, 2014-11-05 21:37:49]

Patchset #1 (id:20001) has been deleted

[hal.canary, 2014-11-05 21:40:09]

This is half of http://crrev.com/694213002 .

Please take a look.

After this lands, I will move us from SkPDFDocument to SkDOcument.

This can wait until after M40.

[Vitaly Buka (NO REVIEWS), 2014-11-05 22:21:38]

https://codereview.chromium.org/704813002/diff/30013/chrome/renderer/printing...
File chrome/renderer/printing/print_web_view_helper_mac.mm (right):

https://codereview.chromium.org/704813002/diff/30013/chrome/renderer/printing...
chrome/renderer/printing/print_web_view_helper_mac.mm:130: if
(params.display_header_footer) {
static_cast

https://codereview.chromium.org/704813002/diff/30013/chrome/renderer/printing...
chrome/renderer/printing/print_web_view_helper_mac.mm:140: scale_factor,
(blink::WebCanvas*)canvas);
static_cast

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
File printing/pdf_metafile_skia.cc (right):

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:37: bool PageOutstanding() { return
current_page_canvas_; }
Can you make it pure struct without methods?
I see no problem with checking data_->current_page_canvas_

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:74: skia::RefPtr<SkPDFDevice> pdf_device =
can you just assing directly into data_->current_page_

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:84: const gfx::Size& page_size, const gfx::Rect&
content_area,
seems running "git cl format" may fix this formatting.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:86: (void)this->StartPage(page_size, content_area,
scale_factor);
if (!StartPage(page_size, content_area, scale_factor))
  return nullptr.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:190: DCHECK(file);
no need DCHECK here. It will crash on file-> anyway.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:210: bool result = this->SaveTo(&file);
no need this->

[hal.canary, 2014-11-05 23:10:58]

https://codereview.chromium.org/704813002/diff/30013/chrome/renderer/printing...
File chrome/renderer/printing/print_web_view_helper_mac.mm (right):

https://codereview.chromium.org/704813002/diff/30013/chrome/renderer/printing...
chrome/renderer/printing/print_web_view_helper_mac.mm:130: if
(params.display_header_footer) {
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> static_cast

Done.

https://codereview.chromium.org/704813002/diff/30013/chrome/renderer/printing...
chrome/renderer/printing/print_web_view_helper_mac.mm:140: scale_factor,
(blink::WebCanvas*)canvas);
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> static_cast

Done.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
File printing/pdf_metafile_skia.cc (right):

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:37: bool PageOutstanding() { return
current_page_canvas_; }
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> Can you make it pure struct without methods?
> I see no problem with checking data_->current_page_canvas_

Done.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:74: skia::RefPtr<SkPDFDevice> pdf_device =
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> can you just assing directly into data_->current_page_

Done.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:84: const gfx::Size& page_size, const gfx::Rect&
content_area,
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> seems running "git cl format" may fix this formatting.

Done.

For some reason I had to `chmod +x buildtools/linux64/clang-format` before this
worked.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:86: (void)this->StartPage(page_size, content_area,
scale_factor);
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> if (!StartPage(page_size, content_area, scale_factor))
>   return nullptr.

Done.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:190: DCHECK(file);
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> no need DCHECK here. It will crash on file-> anyway.

Done.

https://codereview.chromium.org/704813002/diff/30013/printing/pdf_metafile_sk...
printing/pdf_metafile_skia.cc:210: bool result = this->SaveTo(&file);
On 2014/11/05 22:21:38, Vitaly Buka wrote:
> no need this->

Done.

[Vitaly Buka (NO REVIEWS), 2014-11-06 18:59:12]

lgtm

thanks

https://codereview.chromium.org/704813002/diff/50001/chrome/renderer/printing...
File chrome/renderer/printing/print_web_view_helper_mac.mm (right):

https://codereview.chromium.org/704813002/diff/50001/chrome/renderer/printing...
chrome/renderer/printing/print_web_view_helper_mac.mm:131:
PrintHeaderAndFooter(static_cast<blink::WebCanvas*>(canvas),
btw. why do we need cast here at all?
original code seems works just fine

https://codereview.chromium.org/704813002/diff/50001/chrome/renderer/printing...
chrome/renderer/printing/print_web_view_helper_mac.mm:137: scale_factor,
static_cast<blink::WebCanvas*>(canvas));
same

[sgurun-gerrit only, 2014-11-07 00:50:23]

On 2014/11/06 18:59:12, Vitaly Buka wrote:
> lgtm
> 
> thanks
> 
>
https://codereview.chromium.org/704813002/diff/50001/chrome/renderer/printing...
> File chrome/renderer/printing/print_web_view_helper_mac.mm (right):
> 
>
https://codereview.chromium.org/704813002/diff/50001/chrome/renderer/printing...
> chrome/renderer/printing/print_web_view_helper_mac.mm:131:
> PrintHeaderAndFooter(static_cast<blink::WebCanvas*>(canvas),
> btw. why do we need cast here at all?
> original code seems works just fine
> 
>
https://codereview.chromium.org/704813002/diff/50001/chrome/renderer/printing...
> chrome/renderer/printing/print_web_view_helper_mac.mm:137: scale_factor,
> static_cast<blink::WebCanvas*>(canvas));
> same

aw lgtm.

[hal.canary, 2014-11-10 19:52:28]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-11-10 19:52:40]

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

[commit-bot: I haz the power, 2014-11-10 19:56:36]

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

[commit-bot: I haz the power, 2014-11-10 19:56:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hal.canary, 2014-11-10 20:00:18]

halcanary@google.com changed reviewers:
+ tomhudson@google.com

[hal.canary, 2014-11-10 20:00:19]

+tomhudson@google.com

[tomhudson, 2014-11-10 20:01:45]

Skia file deletion LGTM

[tomhudson, 2014-11-10 20:01:51]

The CQ bit was checked by tomhudson@google.com

[hal.canary, 2014-11-10 20:02:13]

The CQ bit was unchecked by halcanary@google.com

[hal.canary, 2014-11-10 20:02:17]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-11-10 20:03:09]

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

[commit-bot: I haz the power, 2014-11-10 20:53:48]

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

[commit-bot: I haz the power, 2014-11-10 20:53:50]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)

[hal.canary, 2014-11-10 21:59:11]

The CQ bit was checked by halcanary@google.com

[commit-bot: I haz the power, 2014-11-10 22:00:48]

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

[commit-bot: I haz the power, 2014-11-10 22:20:18]

Committed patchset #3 (id:70001)

[commit-bot: I haz the power, 2014-11-10 22:21:05]

Patchset 3 (id:??) landed as
https://crrev.com/5be808e0b7478b27304f5a29b47ba1af1de627d2
Cr-Commit-Position: refs/heads/master@{#303522}