Move MediaPlayerAction to WebMediaPlayerAction. Switch to an enum and a...

Move MediaPlayerAction to WebMediaPlayerAction.  Switch to an enum and a
boolean to express the action.

R=ajwong
BUG=10033
TEST=none


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

[awong, 2009-10-06 17:51:52]

couple of small nits.

http://codereview.chromium.org/251103/diff/1/10
File chrome/browser/renderer_host/render_view_host.cc (right):

http://codereview.chromium.org/251103/diff/1/10#newcode1328
Line 1328: delegate_->ShowModalHTMLDialog(
This shouldn't need a linebreak.

http://codereview.chromium.org/251103/diff/1/11
File chrome/browser/renderer_host/render_view_host.h (right):

http://codereview.chromium.org/251103/diff/1/11#newcode305
Line 305: // Send an action to the media player element located at |x|, |y|.
The comment for |x| and |y| should be updated to |location|

http://codereview.chromium.org/251103/diff/1/14
File chrome/common/webkit_param_traits.h (right):

http://codereview.chromium.org/251103/diff/1/14#newcode328
Line 328: WriteParam(m, static_cast<int>(p.type));
I wonder if making p.type an "int" instead of "int32" would be sensible?  It'd
avoid this cast...

[darin (slow to review), 2009-10-06 19:57:50]

Thanks for the review.

http://codereview.chromium.org/251103/diff/1/10
File chrome/browser/renderer_host/render_view_host.cc (right):

http://codereview.chromium.org/251103/diff/1/10#newcode1328
Line 1328: delegate_->ShowModalHTMLDialog(
On 2009/10/06 17:51:52, awong wrote:
> This shouldn't need a linebreak.

Done.

http://codereview.chromium.org/251103/diff/1/11
File chrome/browser/renderer_host/render_view_host.h (right):

http://codereview.chromium.org/251103/diff/1/11#newcode305
Line 305: // Send an action to the media player element located at |x|, |y|.
On 2009/10/06 17:51:52, awong wrote:
> The comment for |x| and |y| should be updated to |location|

Done.

http://codereview.chromium.org/251103/diff/1/14
File chrome/common/webkit_param_traits.h (right):

http://codereview.chromium.org/251103/diff/1/14#newcode328
Line 328: WriteParam(m, static_cast<int>(p.type));
On 2009/10/06 17:51:52, awong wrote:
> I wonder if making p.type an "int" instead of "int32" would be sensible?  It'd
> avoid this cast...

p.type is actually an enum type, so the static_cast to int is needed so that
WriteInt will be selected by the compiler.

[awong, 2009-10-06 20:04:36]

On 2009/10/06 19:57:50, darin wrote:
> Thanks for the review.
> 
> http://codereview.chromium.org/251103/diff/1/10
> File chrome/browser/renderer_host/render_view_host.cc (right):
> 
> http://codereview.chromium.org/251103/diff/1/10#newcode1328
> Line 1328: delegate_->ShowModalHTMLDialog(
> On 2009/10/06 17:51:52, awong wrote:
> > This shouldn't need a linebreak.
> 
> Done.
> 
> http://codereview.chromium.org/251103/diff/1/11
> File chrome/browser/renderer_host/render_view_host.h (right):
> 
> http://codereview.chromium.org/251103/diff/1/11#newcode305
> Line 305: // Send an action to the media player element located at |x|, |y|.
> On 2009/10/06 17:51:52, awong wrote:
> > The comment for |x| and |y| should be updated to |location|
> 
> Done.
> 
> http://codereview.chromium.org/251103/diff/1/14
> File chrome/common/webkit_param_traits.h (right):
> 
> http://codereview.chromium.org/251103/diff/1/14#newcode328
> Line 328: WriteParam(m, static_cast<int>(p.type));
> On 2009/10/06 17:51:52, awong wrote:
> > I wonder if making p.type an "int" instead of "int32" would be sensible? 
It'd
> > avoid this cast...
> 
> p.type is actually an enum type, so the static_cast to int is needed so that
> WriteInt will be selected by the compiler.

Ah, that makes sense.

Did you upload a new patch?  I'm not seeing changes...

[darin (slow to review), 2009-10-06 20:08:11]

> Did you upload a new patch?  I'm not seeing changes...

Sorry, it's uploaded now.  Please have a look.

[awong, 2009-10-06 20:15:24]

LGTM