Fixes Issues #5751 & #22631: NPObject identity

Preserves NPObject identity for objects created in a plugin and passed to JavaScript.

BUG=5751, 22631
TEST=ui_tests w/ the addition of newly added NPObjectIdentityTest.
Committed http://src.chromium.org/viewvc/chrome?view=rev&revision=86893

[jam, 2011-05-20 23:22:22]

thanks a lot for the help :)  just some nits

also, please keep chromium-reviews@chromium.org CCd

http://codereview.chromium.org/7037027/diff/7025/chrome/test/data/npapi/npobj...
File chrome/test/data/npapi/npobject_identity.html (right):

http://codereview.chromium.org/7037027/diff/7025/chrome/test/data/npapi/npobj...
chrome/test/data/npapi/npobject_identity.html:1: 
nit: extra line

http://codereview.chromium.org/7037027/diff/7025/content/plugin/npobject_prox...
File content/plugin/npobject_proxy.cc (right):

http://codereview.chromium.org/7037027/diff/7025/content/plugin/npobject_prox...
content/plugin/npobject_proxy.cc:87: channel->AddMappingForProxy(route_id,
reinterpret_cast<NPObject*>(obj));
no need to cast, if you have an NPObjectWrapper, just do obj->object

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
File content/plugin/plugin_channel_base.cc (right):

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
content/plugin/plugin_channel_base.cc:250: return iter != stub_map_.end() ?
iter->second : -1;
nit: better to return MSG_ROUTING_NONE than -1.  also change callers to use that

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
File content/plugin/plugin_channel_base.h (right):

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
content/plugin/plugin_channel_base.h:64: void AddMappingForProxy(int route_id,
NPObject* object);
nit: we have several proxy/stub types.  can you call this NPObjectProxy instead
of Proxy, to be clear?

[Kelly Norton, 2011-05-23 15:02:56]

http://codereview.chromium.org/7037027/diff/7025/chrome/test/data/npapi/npobj...
File chrome/test/data/npapi/npobject_identity.html (right):

http://codereview.chromium.org/7037027/diff/7025/chrome/test/data/npapi/npobj...
chrome/test/data/npapi/npobject_identity.html:1: 
On 2011/05/20 23:22:22, John Abd-El-Malek wrote:
> nit: extra line

Done.

http://codereview.chromium.org/7037027/diff/7025/content/plugin/npobject_prox...
File content/plugin/npobject_proxy.cc (right):

http://codereview.chromium.org/7037027/diff/7025/content/plugin/npobject_prox...
content/plugin/npobject_proxy.cc:87: channel->AddMappingForProxy(route_id,
reinterpret_cast<NPObject*>(obj));
On 2011/05/20 23:22:22, John Abd-El-Malek wrote:
> no need to cast, if you have an NPObjectWrapper, just do obj->object

Done.

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
File content/plugin/plugin_channel_base.cc (right):

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
content/plugin/plugin_channel_base.cc:250: return iter != stub_map_.end() ?
iter->second : -1;
On 2011/05/20 23:22:22, John Abd-El-Malek wrote:
> nit: better to return MSG_ROUTING_NONE than -1.  also change callers to use
that

Done.

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
File content/plugin/plugin_channel_base.h (right):

http://codereview.chromium.org/7037027/diff/7025/content/plugin/plugin_channe...
content/plugin/plugin_channel_base.h:64: void AddMappingForProxy(int route_id,
NPObject* object);
On 2011/05/20 23:22:22, John Abd-El-Malek wrote:
> nit: we have several proxy/stub types.  can you call this NPObjectProxy
instead
> of Proxy, to be clear?

Done.

[jam, 2011-05-23 16:32:55]

lgtm

[Kelly Norton, 2011-05-23 17:01:24]

Thanks,

Can you land this for me? Or is there now a commit-queue or something (I saw
a commit checkbox on rietveld)?

/kelly

On Mon, May 23, 2011 at 12:32 PM, <jam@chromium.org> wrote:

> lgtm
>
>
> http://codereview.chromium.org/7037027/
>



-- 
If you received this communication by mistake, you are entitled to one free
ice cream cone on me. Simply print out this email including all relevant
SMTP headers and present them at my desk to claim your creamy treat. We'll
have a laugh at my emailing incompetence, and play a game of ping pong.
(offer may not be valid in all States).

[M-A Ruel, 2011-05-25 16:40:25]

This change has no base url so the commit queue cannot handle it.

You need to git svn init & git svn fetch to make use of it.

M-A

[Kelly Norton, 2011-05-25 19:11:49]

I just uploaded a new version of the patch ... how can I tell if I managed
to include a base url?

/kelly

On Wed, May 25, 2011 at 12:40 PM, <maruel@chromium.org> wrote:

> This change has no base url so the commit queue cannot handle it.
>
> You need to git svn init & git svn fetch to make use of it.
>
> M-A
>
>
> http://codereview.chromium.org/7037027/
>



-- 
If you received this communication by mistake, you are entitled to one free
ice cream cone on me. Simply print out this email including all relevant
SMTP headers and present them at my desk to claim your creamy treat. We'll
have a laugh at my emailing incompetence, and play a game of ping pong.
(offer may not be valid in all States).

[M-A Ruel, 2011-05-25 19:15:09]

On 2011/05/25 19:11:49, Kelly Norton wrote:
> I just uploaded a new version of the patch ... how can I tell if I managed
> to include a base url?

base url cannot be changed since it would be a security issue. The easiest:

# Make sure git is correctly initialized
git svn info
# Reset issue number
git cl issue 0
# Upload a new issue, notice the review was done on the previous issue
git cl upload --send-mail -r jam@chromium.org
# Wait for rubberstamp lgtm
# Check the commit box.

M-A

[Kelly Norton, 2011-05-25 20:48:28]

Trying again ...
http://codereview.chromium.org/7068022/

I just used gcl this time as git svn started just hanging and I didn't want
to rebuild the git repo again.

/kelly

On Wed, May 25, 2011 at 3:15 PM, <maruel@chromium.org> wrote:

> On 2011/05/25 19:11:49, Kelly Norton wrote:
>
>> I just uploaded a new version of the patch ... how can I tell if I managed
>> to include a base url?
>>
>
> base url cannot be changed since it would be a security issue. The easiest:
>
> # Make sure git is correctly initialized
> git svn info
> # Reset issue number
> git cl issue 0
> # Upload a new issue, notice the review was done on the previous issue
> git cl upload --send-mail -r jam@chromium.org
> # Wait for rubberstamp lgtm
> # Check the commit box.
>
>
> M-A
>
> http://codereview.chromium.org/7037027/
>



-- 
If you received this communication by mistake, you are entitled to one free
ice cream cone on me. Simply print out this email including all relevant
SMTP headers and present them at my desk to claim your creamy treat. We'll
have a laugh at my emailing incompetence, and play a game of ping pong.
(offer may not be valid in all States).