Cast pointer to uintptr_t instead of ULONG

Cast pointer to uintptr_t instead of ULONG

NPEvent::lParam is defined as uintptr_t which has same width with ULONG on Win32, but they are of different sizes on WIN64. uintptr_t is 8-byte long, while ULONG is still 4-byte long, so it corrupts a 64-bit pointer if casting it to a 32-bit type.

BUG=413535
TEST=square.html (attached in the bug), http://pbskids.org/video (it uses wmode="opaque")
R=ananta@chromium.org, bauerb@chromium.org

Committed: https://crrev.com/8f3f93deca6710d7001415afb3b0822ad91708d1
Cr-Commit-Position: refs/heads/master@{#294917}

[ananta, 2014-09-12 18:25:56]

lgtm

[Xing, 2014-09-12 18:28:12]

This is 64-bit Chrome + NPAPI flash specific issue. It was originally reported
by Adobe Dreamweaver team, and we could reproduce it with 64-bit Chrome build.

I know that NPAPI will be discontinued, so I just came with this simple fix to
help.

[Will Harris, 2014-09-12 22:22:25]

wfh@chromium.org changed reviewers:
+ wfh@chromium.org

[Will Harris, 2014-09-12 22:22:25]

thanks for finding this and fixing it.

https://codereview.chromium.org/571583002/diff/1/content/child/npapi/webplugi...
File content/child/npapi/webplugin_delegate_impl_win.cc (right):

https://codereview.chromium.org/571583002/diff/1/content/child/npapi/webplugi...
content/child/npapi/webplugin_delegate_impl_win.cc:1054: // NOTE: NPAPI is not
64bit safe.  It puts pointers into 32bit values.
this comment is now incorrect since both wParam and lParam are uintptr_t, and
hdc is a HANDLE which is guaranteed to never overflow 32-bits.

[Xing, 2014-09-15 16:57:57]

On 2014/09/12 22:22:25, Will Harris wrote:
> thanks for finding this and fixing it.
> 
>
https://codereview.chromium.org/571583002/diff/1/content/child/npapi/webplugi...
> File content/child/npapi/webplugin_delegate_impl_win.cc (right):
> 
>
https://codereview.chromium.org/571583002/diff/1/content/child/npapi/webplugi...
> content/child/npapi/webplugin_delegate_impl_win.cc:1054: // NOTE: NPAPI is not
> 64bit safe.  It puts pointers into 32bit values.
> this comment is now incorrect since both wParam and lParam are uintptr_t, and
> hdc is a HANDLE which is guaranteed to never overflow 32-bits.

Thanks for reviewing the fix. This is the first Chromium code review from me, I
wonder if there is anything I need to do before the change can be landed?

Since I don't have permission to submit the change, I need your help to land the
fix once the code review is done.

[Bernhard Bauer, 2014-09-15 17:12:48]

On 2014/09/15 16:57:57, Xing wrote:
> On 2014/09/12 22:22:25, Will Harris wrote:
> > thanks for finding this and fixing it.
> > 
> >
>
https://codereview.chromium.org/571583002/diff/1/content/child/npapi/webplugi...
> > File content/child/npapi/webplugin_delegate_impl_win.cc (right):
> > 
> >
>
https://codereview.chromium.org/571583002/diff/1/content/child/npapi/webplugi...
> > content/child/npapi/webplugin_delegate_impl_win.cc:1054: // NOTE: NPAPI is
not
> > 64bit safe.  It puts pointers into 32bit values.
> > this comment is now incorrect since both wParam and lParam are uintptr_t,
and
> > hdc is a HANDLE which is guaranteed to never overflow 32-bits.
> 
> Thanks for reviewing the fix. This is the first Chromium code review from me,
I
> wonder if there is anything I need to do before the change can be landed?

Fix the comment Will pointed out and add yourself to the AUTHORS file.

> Since I don't have permission to submit the change, I need your help to land
the
> fix once the code review is done.

You can check the "commit" checkbox on this CL (but please only after you have
completed the steps above).

[Xing, 2014-09-15 21:19:48]

The CQ bit was checked by xzhang@adobe.com

[Xing, 2014-09-15 21:20:06]

The CQ bit was unchecked by xzhang@adobe.com

[Xing, 2014-09-15 21:20:58]

Updated the incorrect comment according to Will;
Added myself into AUTHORS file

[Xing, 2014-09-15 21:22:03]

Patch 3 is rebase-only, according to Chromium code review process.

[Xing, 2014-09-15 21:22:29]

The CQ bit was checked by xzhang@adobe.com

[Xing, 2014-09-15 21:24:00]

checked "commit" checkbox, according to Bernhard's comments

[commit-bot: I haz the power, 2014-09-15 21:27:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/571583002/40001

[commit-bot: I haz the power, 2014-09-15 23:27:16]

Committed patchset #3 (id:40001) as 54bacd7623e2caa11d862f0f3c50b531b8df66ef

[commit-bot: I haz the power, 2014-09-15 23:32:20]

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