don't need gfx::Canvas and its native context to convert HBITMAP

don't need gfx::Canvas and its native context to convert HBITMAP

Because we can now hold onto the hbitmap for the life of the skia_bitmap, we save a copy over the prev. impl.

BUG=675977
Review-Url: https://codereview.chromium.org/2615683002
Cr-Commit-Position: refs/heads/master@{#441684}
Committed: https://chromium.googlesource.com/chromium/src/+/35b5c4707ee990bd2280d3b43e2d9ea5a6dddcf4

[reed1, 2017-01-04 18:21:32]

reed@google.com changed reviewers:
+ fmalita@chromium.org

[reed1, 2017-01-04 18:21:36]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 18:22:00]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2017-01-04 18:28:03]

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

[reed1, 2017-01-04 18:29:00]

reed@google.com changed reviewers:
+ brianosman@google.com

[reed1, 2017-01-04 18:35:17]

Description was changed from

==========
don't need gfx::Canvas and its native context to convert HBITMAP

BUG=675977
==========

to

==========
don't need gfx::Canvas and its native context to convert HBITMAP

Because we can now hold onto the hbitmap for the life of the skia_bitmap, we
save a copy over the prev. impl.

BUG=675977
==========

[reed1, 2017-01-04 18:36:18]

reed@google.com changed reviewers:
+ dcheng@chromium.org
- halcanary@chromium.org

[commit-bot: I haz the power, 2017-01-04 18:59:34]

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

[commit-bot: I haz the power, 2017-01-04 18:59:36]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[reed1, 2017-01-04 19:19:30]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 19:19:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2017-01-04 19:41:59]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-04 19:42:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2017-01-04 20:25:04]

reed@google.com changed reviewers:
- brianosman@google.com

[reed1, 2017-01-04 20:25:04]

ptal

[commit-bot: I haz the power, 2017-01-04 20:38:13]

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

[commit-bot: I haz the power, 2017-01-04 20:38:14]

Dry run: This issue passed the CQ dry run.

[reed1, 2017-01-04 21:27:35]

reed@google.com changed reviewers:
+ pkasting@chromium.org

[reed1, 2017-01-04 21:27:35]

need owner for ui/base/clipboard/clipboard_win.cc

[dcheng, 2017-01-04 22:25:36]

Seems reasonable to me, just one comment and a question

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_win.cc (right):

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:622: HDC hdc = CreateCompatibleDC(NULL);
Nit: nullptr, and use ScopedCreateDC from base/win

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:623: skia::InitializeDC(hdc);
Just curious: do we need to do this if we're just going to call
SetDIBitsToDevice?

[reed1, 2017-01-05 02:24:47]

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_win.cc (right):

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:622: HDC hdc = CreateCompatibleDC(NULL);
On 2017/01/04 22:25:36, dcheng wrote:
> Nit: nullptr, and use ScopedCreateDC from base/win

I would use it like this? ScopedCreateDC scoped(::CreateCompatibleDC(NULL));

That classes doesn't seems to do anything with selecting in and out of my
hbitmap. Is it applicable in my case?

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:623: skia::InitializeDC(hdc);
On 2017/01/04 22:25:36, dcheng wrote:
> Just curious: do we need to do this if we're just going to call
> SetDIBitsToDevice?

I need to draw 'bitmap' into my dst_hbitmap. Don't I need to create a DC around
dst_hbitmap to do that?

[dcheng, 2017-01-05 02:39:50]

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_win.cc (right):

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:622: HDC hdc = CreateCompatibleDC(NULL);
On 2017/01/05 02:24:46, reed1 wrote:
> On 2017/01/04 22:25:36, dcheng wrote:
> > Nit: nullptr, and use ScopedCreateDC from base/win
> 
> I would use it like this? ScopedCreateDC scoped(::CreateCompatibleDC(NULL));
> 
> That classes doesn't seems to do anything with selecting in and out of my
> hbitmap. Is it applicable in my case?
> 

Yes, it would still need to be selected, but it would automatically handle the
DeleteDC call.

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:623: skia::InitializeDC(hdc);
On 2017/01/05 02:24:46, reed1 wrote:
> On 2017/01/04 22:25:36, dcheng wrote:
> > Just curious: do we need to do this if we're just going to call
> > SetDIBitsToDevice?
> 
> I need to draw 'bitmap' into my dst_hbitmap. Don't I need to create a DC
around
> dst_hbitmap to do that?

I think the DC should already be initialized; it looks like InitializeDC just
sets up some defaults that we shouldn't have to use anyway.

[reed1, 2017-01-05 03:16:08]

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_win.cc (right):

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:623: skia::InitializeDC(hdc);
On 2017/01/05 02:39:50, dcheng wrote:
> On 2017/01/05 02:24:46, reed1 wrote:
> > On 2017/01/04 22:25:36, dcheng wrote:
> > > Just curious: do we need to do this if we're just going to call
> > > SetDIBitsToDevice?
> > 
> > I need to draw 'bitmap' into my dst_hbitmap. Don't I need to create a DC
> around
> > dst_hbitmap to do that?
> 
> I think the DC should already be initialized; it looks like InitializeDC just
> sets up some defaults that we shouldn't have to use anyway.

Ah, I misinterpreted your question. Got it about InitializeDC. Thanks.

[reed1, 2017-01-05 15:49:44]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 15:49:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2017-01-05 16:09:57]

The CQ bit was checked by reed@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 16:10:17]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[reed1, 2017-01-05 16:49:09]

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
File ui/base/clipboard/clipboard_win.cc (right):

https://codereview.chromium.org/2615683002/diff/40001/ui/base/clipboard/clipb...
ui/base/clipboard/clipboard_win.cc:622: HDC hdc = CreateCompatibleDC(NULL);
On 2017/01/05 02:39:50, dcheng wrote:
> On 2017/01/05 02:24:46, reed1 wrote:
> > On 2017/01/04 22:25:36, dcheng wrote:
> > > Nit: nullptr, and use ScopedCreateDC from base/win
> > 
> > I would use it like this? ScopedCreateDC scoped(::CreateCompatibleDC(NULL));
> > 
> > That classes doesn't seems to do anything with selecting in and out of my
> > hbitmap. Is it applicable in my case?
> > 
> 
> Yes, it would still need to be selected, but it would automatically handle the
> DeleteDC call.

Done.

[dcheng, 2017-01-05 16:58:23]

lgtm

[commit-bot: I haz the power, 2017-01-05 17:06:54]

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

[commit-bot: I haz the power, 2017-01-05 17:06:55]

Dry run: This issue passed the CQ dry run.

[reed1, 2017-01-05 17:13:27]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2017-01-05 17:13:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 17:20:34]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1483636407761700,
"parent_rev": "73af4f477cdc6b962b8fd069353ea8df80af8491", "commit_rev":
"35b5c4707ee990bd2280d3b43e2d9ea5a6dddcf4"}

[commit-bot: I haz the power, 2017-01-05 17:21:03]

Description was changed from

==========
don't need gfx::Canvas and its native context to convert HBITMAP

Because we can now hold onto the hbitmap for the life of the skia_bitmap, we
save a copy over the prev. impl.

BUG=675977
==========

to

==========
don't need gfx::Canvas and its native context to convert HBITMAP

Because we can now hold onto the hbitmap for the life of the skia_bitmap, we
save a copy over the prev. impl.

BUG=675977

Review-Url: https://codereview.chromium.org/2615683002
Cr-Commit-Position: refs/heads/master@{#441684}
Committed:
https://chromium.googlesource.com/chromium/src/+/35b5c4707ee990bd2280d3b43e2d...
==========

[commit-bot: I haz the power, 2017-01-05 17:21:05]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/35b5c4707ee990bd2280d3b43e2d...