Make WebMediaPlayerClient inheritance private in HTMLMediaElement

Make WebMediaPlayerClient inheritance private in HTMLMediaElement

Make WebMediaPlayerClient inheritance private in HTMLMediaElement
by passing WebMediaPlayerClient to createWebMediaPlayer

BUG=350571
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=199731

[Srirama, 2015-07-24 10:36:54]

Patchset #2 (id:20001) has been deleted

[Srirama, 2015-07-24 12:15:28]

srirama.m@samsung.com changed reviewers:
+ philipj@opera.com

[Srirama, 2015-07-24 12:15:29]

PTAL.

[Srirama, 2015-07-30 06:01:15]

Gentle reminder.
Should we wait till https://code.google.com/p/chromium/issues/detail?id=514178
is fixed?

[philipj_slow, 2015-07-30 08:59:38]

Sorry, I overlooked this.

It's pretty clear now that it's not going to be possible to hide
HTMLMediaElement itself from web, as that dependency is necessary so that
modules/encryptedmedia can get the supplement and get the
WebMediaPlayerEncryptedMediaClient and WebContentDecryptionModule.

Given that the dependency cannot be broken entirely, I'm thinking that perhaps
we should instead pass both WebMediaPlayerClient* and HTMLMediaElement& to
createWebMediaPlayer:

PassOwnPtr<WebMediaPlayer> createWebMediaPlayer(HTMLMediaElement&, const
WebURL&, WebMediaPlayerClient*);

That would also allow the inheritance to be made private, and the first argument
a reference somehow documents that you shouldn't hold onto it.

[philipj_slow, 2015-07-30 09:00:32]

On 2015/07/30 06:01:15, Srirama wrote:
> Should we wait till https://code.google.com/p/chromium/issues/detail?id=514178
> is fixed?

No need I think, if your CL was the cause it's more likely that it can be fixed
than that it will all have to be reverted.

[Srirama, 2015-07-30 10:41:57]

On 2015/07/30 09:00:32, philipj wrote:
> On 2015/07/30 06:01:15, Srirama wrote:
> > Should we wait till
https://code.google.com/p/chromium/issues/detail?id=514178
> > is fixed?
> 
> No need I think, if your CL was the cause it's more likely that it can be
fixed
> than that it will all have to be reverted.

Thanks Philip. Done the changes.
please check if I have captured your comments properly.

[philipj_slow, 2015-07-30 11:17:41]

LGTM % nit

https://codereview.chromium.org/1257583003/diff/80001/Source/core/html/HTMLMe...
File Source/core/html/HTMLMediaElement.h (right):

https://codereview.chromium.org/1257583003/diff/80001/Source/core/html/HTMLMe...
Source/core/html/HTMLMediaElement.h:69: class CORE_EXPORT HTMLMediaElement :
public HTMLElement, public WillBeHeapSupplementable<HTMLMediaElement>, private
WebMediaPlayerClient, public ActiveDOMObject {
Nit: now that it's private, and it starts with a W, I think it would be nice to
put this after ActiveDOMObject.

[Srirama, 2015-07-30 12:59:22]

The CQ bit was checked by srirama.m@samsung.com

[Srirama, 2015-07-30 12:59:22]

The patchset sent to the CQ was uploaded after l-g-t-m from philipj@opera.com
Link to the patchset: https://codereview.chromium.org/1257583003/#ps100001
(title: "address nit")

[commit-bot: I haz the power, 2015-07-30 12:59:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1257583003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1257583003/100001

[Srirama, 2015-07-30 12:59:41]

https://codereview.chromium.org/1257583003/diff/80001/Source/core/html/HTMLMe...
File Source/core/html/HTMLMediaElement.h (right):

https://codereview.chromium.org/1257583003/diff/80001/Source/core/html/HTMLMe...
Source/core/html/HTMLMediaElement.h:69: class CORE_EXPORT HTMLMediaElement :
public HTMLElement, public WillBeHeapSupplementable<HTMLMediaElement>, private
WebMediaPlayerClient, public ActiveDOMObject {
On 2015/07/30 11:17:41, philipj wrote:
> Nit: now that it's private, and it starts with a W, I think it would be nice
to
> put this after ActiveDOMObject.

Done.

[commit-bot: I haz the power, 2015-07-30 13:02:07]

Committed patchset #5 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199731