Move CdmResultPromise to media/blink.

Move CdmResultPromise to media/blink.

BUG=422730
Committed: https://crrev.com/f440ca2e146138fd2ecdf71f37b4432cf61ce09e
Cr-Commit-Position: refs/heads/master@{#299912}

[xhwang, 2014-10-14 06:38:10]

Patchset #2 (id:20001) has been deleted

[xhwang, 2014-10-14 07:41:52]

Patchset #3 (id:60001) has been deleted

[xhwang, 2014-10-14 16:23:13]

xhwang@chromium.org changed reviewers:
+ ddorwin@chromium.org, jrummell@chromium.org

[xhwang, 2014-10-14 16:23:14]

See bug for the background of this CL.

In PS1, I copied files over w/o any modification so that we have nice diff in
later PS.

In PS2, I made the real move. But on some platforms the linker cannot find the
CdmResultPromise<media::KeyIdsVector>::CdmResultPromise(). I cannot repro this
on my Android or Linux check out. I tried several ways and did some research and
still don't understand why. Note that we do exactly the same for
CdmResultPromise<>::CdmResultPromise(). Through the investigation, I feel there
are many imcompatibility issues among all kinds of compilers/linkers. Using
implicit instantiation (putting definitions in the .h file) is always the
simplest (more compatible) way. If you know how to fix this please let me know!~

In PS3, I move CdmResultPromise definitions into the header file. As part of
that, I split NewSessionCdmResultPromise into a separate file. As part of that,
I move the UMA reporting code into another separate file :(

PTAL

[xhwang, 2014-10-15 23:34:13]

Kindly ping...

[xhwang, 2014-10-15 23:47:45]

xhwang@chromium.org changed required reviewers:
+ jrummell@chromium.org

[jrummell, 2014-10-16 00:20:16]

lgtm w/nits

https://codereview.chromium.org/651113002/diff/100001/content/renderer/media/...
File content/renderer/media/cdm_result_promise.cc (left):

https://codereview.chromium.org/651113002/diff/100001/content/renderer/media/...
content/renderer/media/cdm_result_promise.cc:158: template class
CdmResultPromise<media::KeyIdsVector>;
Would doing this in cdm_result_promise_helper.cc help with the need for
"inline"? Probably not worth changing since you have this working across all
systems already.

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
File media/blink/cdm_result_promise_helper.cc (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
media/blink/cdm_result_promise_helper.cc:13: media::MediaKeys::Exception
exception_code) {
ditto for media:: here and below.

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
File media/blink/cdm_result_promise_helper.h (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
media/blink/cdm_result_promise_helper.h:32: media::MediaKeys::Exception
exception_code);
media:: not needed.

https://codereview.chromium.org/651113002/diff/100001/media/blink/media_blink...
File media/blink/media_blink.gyp (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/media_blink...
media/blink/media_blink.gyp:40: 'cache_util.cc',
Should you move cache_util.* up to match what you did in the .gn file?

https://codereview.chromium.org/651113002/diff/100001/media/blink/new_session...
File media/blink/new_session_cdm_result_promise.h (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/new_session...
media/blink/new_session_cdm_result_promise.h:26: : public
media::CdmPromiseTemplate<std::string> {
ditto on media::

[xhwang, 2014-10-16 03:44:21]

comments addressed

[xhwang, 2014-10-16 03:46:34]

Patchset #5 (id:120001) has been deleted

[xhwang, 2014-10-16 04:25:44]

https://codereview.chromium.org/651113002/diff/100001/content/renderer/media/...
File content/renderer/media/cdm_result_promise.cc (left):

https://codereview.chromium.org/651113002/diff/100001/content/renderer/media/...
content/renderer/media/cdm_result_promise.cc:158: template class
CdmResultPromise<media::KeyIdsVector>;
On 2014/10/16 00:20:16, jrummell wrote:
> Would doing this in cdm_result_promise_helper.cc help with the need for
> "inline"? Probably not worth changing since you have this working across all
> systems already.

I tried many ways of explicit instantiation but none of them work...

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
File media/blink/cdm_result_promise_helper.cc (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
media/blink/cdm_result_promise_helper.cc:13: media::MediaKeys::Exception
exception_code) {
On 2014/10/16 00:20:16, jrummell wrote:
> ditto for media:: here and below.

Done.

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
File media/blink/cdm_result_promise_helper.h (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/cdm_result_...
media/blink/cdm_result_promise_helper.h:32: media::MediaKeys::Exception
exception_code);
On 2014/10/16 00:20:16, jrummell wrote:
> media:: not needed.

Done.

https://codereview.chromium.org/651113002/diff/100001/media/blink/media_blink...
File media/blink/media_blink.gyp (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/media_blink...
media/blink/media_blink.gyp:40: 'cache_util.cc',
On 2014/10/16 00:20:16, jrummell wrote:
> Should you move cache_util.* up to match what you did in the .gn file?

Done.

https://codereview.chromium.org/651113002/diff/100001/media/blink/new_session...
File media/blink/new_session_cdm_result_promise.h (right):

https://codereview.chromium.org/651113002/diff/100001/media/blink/new_session...
media/blink/new_session_cdm_result_promise.h:26: : public
media::CdmPromiseTemplate<std::string> {
On 2014/10/16 00:20:16, jrummell wrote:
> ditto on media::

Done.

[xhwang, 2014-10-16 16:43:11]

ddorwin: Do you also want to take a look?

[ddorwin, 2014-10-16 16:48:25]

On 2014/10/16 16:43:11, xhwang wrote:
> ddorwin: Do you also want to take a look?

No. Go ahead.

[xhwang, 2014-10-16 16:58:41]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-10-16 17:00:00]

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

[commit-bot: I haz the power, 2014-10-16 17:30:01]

Committed patchset #6 (id:160001)

[commit-bot: I haz the power, 2014-10-16 17:30:41]

Patchset 6 (id:??) landed as
https://crrev.com/f440ca2e146138fd2ecdf71f37b4432cf61ce09e
Cr-Commit-Position: refs/heads/master@{#299912}