Restore RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl.

Restore RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl 

This interface is part of the prefixed API only and should be removed
with it.

BUG=402536, 343413
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187168

[MRV, 2014-09-25 10:25:38]

mohan.reddy@samsung.com changed reviewers:
+ abarth@chromium.org

[MRV, 2014-09-25 10:25:39]

PTAL

[ddorwin, 2014-09-25 16:45:25]

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

[ddorwin, 2014-09-25 16:45:26]

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaError.idl
File Source/core/html/MediaError.idl (right):

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaError....
Source/core/html/MediaError.idl:34: [DeprecateAs=MediaErrorEncrypted] const
unsigned short MEDIA_ERR_ENCRYPTED = 5;
Instead, please replace with PrefixedEncryptedMedia. Alternatively, I'm fine
with just removing it at this point - it's been deprecated for a while and I
don't think anyone ever used the constant.

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaKeyErr...
File Source/core/html/MediaKeyError.idl (right):

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaKeyErr...
Source/core/html/MediaKeyError.idl:27: WillBeGarbageCollected,
Please instead replace with PrefixedEncryptedMedia. See the BUG for details.

[MRV, 2014-09-26 02:57:16]

Dear Mr. ddorwin,
Thanks for your feedback, I have incorporated changes suggested.
PTAL.

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaError.idl
File Source/core/html/MediaError.idl (right):

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaError....
Source/core/html/MediaError.idl:34: [DeprecateAs=MediaErrorEncrypted] const
unsigned short MEDIA_ERR_ENCRYPTED = 5;
On 2014/09/25 16:45:26, ddorwin wrote:
> Instead, please replace with PrefixedEncryptedMedia. Alternatively, I'm fine
> with just removing it at this point - it's been deprecated for a while and I
> don't think anyone ever used the constant.

Done.

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaKeyErr...
File Source/core/html/MediaKeyError.idl (right):

https://codereview.chromium.org/602063003/diff/1/Source/core/html/MediaKeyErr...
Source/core/html/MediaKeyError.idl:27: WillBeGarbageCollected,
On 2014/09/25 16:45:26, ddorwin wrote:
> Please instead replace with PrefixedEncryptedMedia. See the BUG for details.

Done.

[ddorwin, 2014-09-26 03:34:01]

lgtm

[MRV, 2014-09-26 03:52:25]

On 2014/09/26 03:34:01, ddorwin wrote:
> lgtm

Thanks Mr. ddorwin for LGTM.

[MRV, 2014-09-26 03:52:30]

The CQ bit was checked by mohan.reddy@samsung.com

[commit-bot: I haz the power, 2014-09-26 03:53:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/602063003/20001

[commit-bot: I haz the power, 2014-09-26 04:01:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-26 04:01:14]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/1...)

[ddorwin, 2014-09-26 04:08:28]

I'm not an OWNER for any of these files, so you still need another LGTM. abarth
is pretty busy, so you might want to pick another.

[MRV, 2014-09-26 04:56:38]

mohan.reddy@samsung.com changed reviewers:
+ dglazkov@chromium.org

[MRV, 2014-09-26 04:57:31]

On 2014/09/26 04:08:28, ddorwin wrote:
> I'm not an OWNER for any of these files, so you still need another LGTM.
abarth
> is pretty busy, so you might want to pick another.

Thanks Mr. ddorwin for clarification.
@Mr. dglazkov, could you review my patch.

[ddorwin, 2014-09-26 21:42:14]

ddorwin@chromium.org changed reviewers:
- abarth@chromium.org, dglazkov@chromium.org

[ddorwin, 2014-09-26 21:42:14]

You should add 418260 to BUG=. However, as noted in that bug, I'm not sure that
unprefixed EME is ready to have the error removed, so please hold off on this
removal until that bug is addressed.

[ddorwin, 2014-09-26 21:47:33]

Not lgtm until that bug is fixed. Once it is, you should be able to rebase and
land this. Thanks.

[MRV, 2014-09-29 03:08:45]

On 2014/09/26 21:47:33, ddorwin wrote:
> Not lgtm until that bug is fixed. Once it is, you should be able to rebase and
> land this. Thanks.

Dear Mr. ddorwin, thanks for your feedback.
I'll wait until 418260 gets resolved.

[ddorwin, 2014-12-09 21:48:53]

On 2014/09/29 03:08:45, MRV wrote:
> On 2014/09/26 21:47:33, ddorwin wrote:
> > Not lgtm until that bug is fixed. Once it is, you should be able to rebase
and
> > land this. Thanks.
> 
> Dear Mr. ddorwin, thanks for your feedback.
> I'll wait until 418260 gets resolved.

As of Blink revision 186808, bug 418260 should be fixed, so you can rebase and
re-upload this CL. Thanks.

[MRV, 2014-12-11 11:23:45]

On 2014/12/09 21:48:53, ddorwin wrote:
> On 2014/09/29 03:08:45, MRV wrote:
> > On 2014/09/26 21:47:33, ddorwin wrote:
> > > Not lgtm until that bug is fixed. Once it is, you should be able to rebase
> and
> > > land this. Thanks.
> > 
> > Dear Mr. ddorwin, thanks for your feedback.
> > I'll wait until 418260 gets resolved.
> 
> As of Blink revision 186808, bug 418260 should be fixed, so you can rebase and
> re-upload this CL. Thanks.

Thanks Mr. ddorwin, thanks for the update, i started looking into it, but
heeyoun.lee as worked and closed the same in the
https://codereview.chromium.org/580263004/
So closing this issue again.

[ddorwin, 2014-12-11 19:30:52]

On 2014/12/11 11:23:45, MRV wrote:
> On 2014/12/09 21:48:53, ddorwin wrote:
> > On 2014/09/29 03:08:45, MRV wrote:
> > > On 2014/09/26 21:47:33, ddorwin wrote:
> > > > Not lgtm until that bug is fixed. Once it is, you should be able to
rebase
> > and
> > > > land this. Thanks.
> > > 
> > > Dear Mr. ddorwin, thanks for your feedback.
> > > I'll wait until 418260 gets resolved.
> > 
> > As of Blink revision 186808, bug 418260 should be fixed, so you can rebase
and
> > re-upload this CL. Thanks.
> 
> Thanks Mr. ddorwin, thanks for the update, i started looking into it, but
> heeyoun.lee as worked and closed the same in the
> https://codereview.chromium.org/580263004/
> So closing this issue again.

Thanks for bringing that CL to my attention. As I commented in it, it is
incorrect for Media*.idl. Your change in those two is correct. I think if you
rebase and keep your changes there, you'll have just the two IDL files, which
will be correct.

[MRV, 2014-12-12 06:58:49]

On 2014/12/11 19:30:52, ddorwin wrote:
> On 2014/12/11 11:23:45, MRV wrote:
> > On 2014/12/09 21:48:53, ddorwin wrote:
> > > On 2014/09/29 03:08:45, MRV wrote:
> > > > On 2014/09/26 21:47:33, ddorwin wrote:
> > > > > Not lgtm until that bug is fixed. Once it is, you should be able to
> rebase
> > > and
> > > > > land this. Thanks.
> > > > 
> > > > Dear Mr. ddorwin, thanks for your feedback.
> > > > I'll wait until 418260 gets resolved.
> > > 
> > > As of Blink revision 186808, bug 418260 should be fixed, so you can rebase
> and
> > > re-upload this CL. Thanks.
> > 
> > Thanks Mr. ddorwin, thanks for the update, i started looking into it, but
> > heeyoun.lee as worked and closed the same in the
> > https://codereview.chromium.org/580263004/
> > So closing this issue again.
> 
> Thanks for bringing that CL to my attention. As I commented in it, it is
> incorrect for Media*.idl. Your change in those two is correct. I think if you
> rebase and keep your changes there, you'll have just the two IDL files, which
> will be correct.

Mr. ddorwin,
Thanks for your feedback.
I have made the changes as suggested.
PTAL.

[ddorwin, 2014-12-12 17:55:46]

It looks like one use was already taken care of. See the comments. Just drop
that file from this CL. Also, I'd change the description to something like the
following.

Restore RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl.

MediaKeyError is only present in prefixed EME.

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
File Source/core/html/MediaError.idl (right):

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
Source/core/html/MediaError.idl:34: [RuntimeEnabled=PrefixedEncryptedMedia]
philipj removed the value a couple weeks ago in
https://codereview.chromium.org/724573004, so this is no longer needed.

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaKe...
File Source/core/html/MediaKeyError.idl (right):

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaKe...
Source/core/html/MediaKeyError.idl:27: RuntimeEnabled=PrefixedEncryptedMedia,
This is the one remaining piece from this CL. :)

[philipj_slow, 2014-12-12 20:42:12]

philipj@opera.com changed reviewers:
+ philipj@opera.com

[philipj_slow, 2014-12-12 20:42:13]

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
File Source/core/html/MediaError.idl (right):

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
Source/core/html/MediaError.idl:34: [RuntimeEnabled=PrefixedEncryptedMedia]
On 2014/12/12 17:55:46, ddorwin wrote:
> philipj removed the value a couple weeks ago in
> https://codereview.chromium.org/724573004, so this is no longer needed.

Right, it would actually apply to the code attribute below, which would be
bad...

[MRV, 2014-12-15 03:16:18]

Thanks Mr.ddorwin and philipj for your feedback.
I have modified patch as per your suggestions.
PTAL

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
File Source/core/html/MediaError.idl (right):

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
Source/core/html/MediaError.idl:34: [RuntimeEnabled=PrefixedEncryptedMedia]
On 2014/12/12 17:55:46, ddorwin wrote:
> philipj removed the value a couple weeks ago in
> https://codereview.chromium.org/724573004, so this is no longer needed.

Done.

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaEr...
Source/core/html/MediaError.idl:34: [RuntimeEnabled=PrefixedEncryptedMedia]
On 2014/12/12 20:42:13, philipj wrote:
> On 2014/12/12 17:55:46, ddorwin wrote:
> > philipj removed the value a couple weeks ago in
> > https://codereview.chromium.org/724573004, so this is no longer needed.
> 
> Right, it would actually apply to the code attribute below, which would be
> bad...

Done.

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaKe...
File Source/core/html/MediaKeyError.idl (right):

https://codereview.chromium.org/602063003/diff/60001/Source/core/html/MediaKe...
Source/core/html/MediaKeyError.idl:27: RuntimeEnabled=PrefixedEncryptedMedia,
On 2014/12/12 17:55:46, ddorwin wrote:
> This is the one remaining piece from this CL. :)

Done.

[philipj_slow, 2014-12-15 09:11:10]

LGTM, but please improve the commit message. It says "Restore
RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl." twice and then an
unfinished sentence "To restore the functionality of PrefixedEncryptedMedia"

Remove the duplicate line and simply state that this interface is part of the
prefixed API only and should be removed together with it.

[MRV, 2014-12-15 11:58:17]

On 2014/12/15 09:11:10, philipj wrote:
> LGTM, but please improve the commit message. It says "Restore
> RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl." twice and then an
> unfinished sentence "To restore the functionality of PrefixedEncryptedMedia"
> 
> Remove the duplicate line and simply state that this interface is part of the
> prefixed API only and should be removed together with it.

Thanks philipj for review, I have updated as per your suggestions.

[MRV, 2014-12-15 11:58:25]

The CQ bit was checked by mohan.reddy@samsung.com

[commit-bot: I haz the power, 2014-12-15 11:59:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/602063003/80001

[commit-bot: I haz the power, 2014-12-15 11:59:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-15 11:59:32]

A disapproval has been posted by following reviewers: ddorwin@chromium.org.
Please make sure to get positive review before another CQ attempt.

[philipj_slow, 2014-12-15 14:45:58]

That description is still problematic, and far exceeds the 72 character limit
that git cl upload suggests. I can't edit the description for you, so paste
exactly this:

Restore RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl 

This interface is part of the prefixed API only and should be removed
together with it.

BUG=402536, 343413

[ddorwin, 2014-12-15 17:56:01]

The CQ bit was checked by ddorwin@chromium.org

[ddorwin, 2014-12-15 17:56:01]

lgtm

I copied the description (and removed "together"). Thanks MRV!

[commit-bot: I haz the power, 2014-12-15 17:56:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/602063003/80001

[MRV, 2014-12-15 18:00:28]

On 2014/12/15 14:45:58, philipj wrote:
> That description is still problematic, and far exceeds the 72 character limit
> that git cl upload suggests. I can't edit the description for you, so paste
> exactly this:
> 
> Restore RuntimeEnabled=PrefixedEncryptedMedia to MediaKeyError.idl 
> 
> This interface is part of the prefixed API only and should be removed
> together with it.
> 
> BUG=402536, 343413

Thanks philipj for suggestion.
Due browser issue I was unable to notice the issue.
I updated commit message.
PTAL.

[MRV, 2014-12-15 18:03:25]

On 2014/12/15 17:56:01, ddorwin wrote:
> lgtm
> 
> I copied the description (and removed "together"). Thanks MRV!

Thanks mr.ddorwin.

[commit-bot: I haz the power, 2014-12-15 19:22:22]

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