Gears File Drag Drop

r24899 now keys the child security policy database based on
renderer id, rather than renderer process id.  Update gears
drag drop to use the renderer id for the file policy access
checks.

BUG=7995

[noel.gordon_gmail.com, 2009-09-14 13:18:06]

Gears file drag drop fails from dev channel 4.0.206.1 onwards due to r24899, so
let's fix it.

[brettw, 2009-09-14 14:14:30]

http://codereview.chromium.org/195079/diff/1/6
File chrome/plugin/webplugin_proxy.h (right):

http://codereview.chromium.org/195079/diff/1/6#newcode91
Line 91: int GetRendererProcessId();
This function doesn't look like it's used anymore. Can you remove it?

[darin (slow to review), 2009-09-14 16:30:55]

I defer to brettw to review this CL.

[noel.gordon_gmail.com, 2009-09-15 00:23:15]

On 2009/09/14 14:14:30, brettw wrote:
> http://codereview.chromium.org/195079/diff/1/6
> File chrome/plugin/webplugin_proxy.h (right):
> 
> http://codereview.chromium.org/195079/diff/1/6#newcode91
> Line 91: int GetRendererProcessId();
> This function doesn't look like it's used anymore. Can you remove it?

Done.

[brettw, 2009-09-15 00:37:59]

Sorry I didn't notice this stuff before, I have a few more comments.

http://codereview.chromium.org/195079/diff/5001/5008
File chrome/plugin/plugin_channel.cc (right):

http://codereview.chromium.org/195079/diff/5001/5008#newcode46
Line 46: renderer_id_(0),
Can you use -1 here instead, which ChildProcessInfo uses?

http://codereview.chromium.org/195079/diff/5001/5007
File chrome/plugin/plugin_thread.cc (right):

http://codereview.chromium.org/195079/diff/5001/5007#newcode115
Line 115: channel->set_renderer_id(renderer_id);
It looks to me that this might be settable inside
PluginChannel::GetPluginChannel which means that you don't have to expose a
setter at all. Is this correct?

[noel.gordon_gmail.com, 2009-09-15 00:59:48]

On 2009/09/15 00:37:59, brettw wrote:
> Sorry I didn't notice this stuff before, I have a few more comments.

np.
 
> http://codereview.chromium.org/195079/diff/5001/5008
> File chrome/plugin/plugin_channel.cc (right):
> 
> http://codereview.chromium.org/195079/diff/5001/5008#newcode46
> Line 46: renderer_id_(0),
> Can you use -1 here instead, which ChildProcessInfo uses?

Done.
 
> http://codereview.chromium.org/195079/diff/5001/5007
> File chrome/plugin/plugin_thread.cc (right):
> 
> http://codereview.chromium.org/195079/diff/5001/5007#newcode115
> Line 115: channel->set_renderer_id(renderer_id);
> It looks to me that this might be settable inside
> PluginChannel::GetPluginChannel which means that you don't have to expose a
> setter at all. Is this correct?

Yes, and you could pass off_the_record to GetPluginChannel() and
remove its setter also.  I wrote it this way to be consistent
with the off_the_record setter.  Not sure which way is better.

[brettw, 2009-09-15 01:11:15]

On Mon, Sep 14, 2009 at 5:59 PM,  <noel.gordon@gmail.com> wrote:
> On 2009/09/15 00:37:59, brettw wrote:
>>
>> Sorry I didn't notice this stuff before, I have a few more comments.
>
> np.
>
>> http://codereview.chromium.org/195079/diff/5001/5008
>> File chrome/plugin/plugin_channel.cc (right):
>
>> http://codereview.chromium.org/195079/diff/5001/5008#newcode46
>> Line 46: renderer_id_(0),
>> Can you use -1 here instead, which ChildProcessInfo uses?
>
> Done.
>
>> http://codereview.chromium.org/195079/diff/5001/5007
>> File chrome/plugin/plugin_thread.cc (right):
>
>> http://codereview.chromium.org/195079/diff/5001/5007#newcode115
>> Line 115: channel->set_renderer_id(renderer_id);
>> It looks to me that this might be settable inside
>> PluginChannel::GetPluginChannel which means that you don't have to expos=
e
>> a
>> setter at all. Is this correct?
>
> Yes, and you could pass off_the_record to GetPluginChannel() and
> remove its setter also. =A0I wrote it this way to be consistent
> with the off_the_record setter. =A0Not sure which way is better.

I see. I was figuring since we're already passing in the renderer ID,
it makes the most sense to initialize it there, but your point is also
good. If you don't initialize it in the getter, can you comment that
you must call set_renderer_id before using?

LGTM otherwise.

[noel.gordon_gmail.com, 2009-09-15 14:59:50]

Works for me also, let's initialize in the getter.  Saves a
comment and removes a file from the CL.  New snapshot ...

[brettw, 2009-09-16 02:14:28]

On 2009/09/15 14:59:50, noel wrote:
> Works for me also, let's initialize in the getter.  Saves a
> comment and removes a file from the CL.  New snapshot ...

LGTM. When I did "LGTM" above, that means you can actually just make that change
and check in without doing another round. Sorry for the confusion.

[noel.gordon_gmail.com, 2009-09-16 02:33:40]

np. michael offerred to submit it for me.

[michaeln, 2009-09-16 21:37:20]

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=26397