Add new media request result MEDIA_DEVICE_FAILED_DUE_TO_SHUTDOWN.

Add new media request result MEDIA_DEVICE_FAILED_DUE_TO_SHUTDOWN.

Use it at appropriate places.
Also add corresponding UMA histogram value.

BUG=416233
Committed: https://crrev.com/e544e1309f6264ce005e4f89dc67fbe81b0b8d65
Cr-Commit-Position: refs/heads/master@{#296809}

[Henrik Grunell, 2014-09-20 08:53:54]

grunell@chromium.org changed reviewers:
+ tommi@chromium.org

[Henrik Grunell, 2014-09-20 08:58:57]

grunell@chromium.org changed reviewers:
+ avi@chromium.org, fsamuel@chromium.org, isherman@chromium.org,
michaelbai@chromium.org

[Henrik Grunell, 2014-09-20 08:58:58]

tommi@: Everything. In particular
chrome/browser/media/media_stream_devices_controller.cc
content/browser/renderer_host/media/media_stream_ui_proxy.cc

avi@: content/* except the above.

isherman@: tools/metrics/histograms/histograms.xml

michaelbai@:  android_webview/native/aw_web_contents_delegate.cc

fsamuel@: extensions/browser/guest_view/web_view/web_view_permission_helper.cc

[Henrik Grunell, 2014-09-20 09:01:43]

I have some outstanding questions mail wise about if these cases can happen
apart from when shutting down. It's good if everyone can verify that in the CL.
Otherwise the naming must change.

[Avi (use Gerrit), 2014-09-20 13:03:15]

Content non-media aspects LGTM, though you really need a bug number in the
description.

[Henrik Grunell, 2014-09-20 13:05:22]

On 2014/09/20 13:03:15, Avi wrote:
> Content non-media aspects LGTM, though you really need a bug number in the
> description.

Added bug.

[tommi (sloooow) - chröme, 2014-09-20 14:30:36]

lgtm

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
File extensions/browser/guest_view/web_view/web_view_permission_helper.cc
(right):

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:199: if
(!web_view_guest()->embedder_web_contents()->GetDelegate())
is this a fix for another bug?

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:223:
scoped_ptr<content::MediaStreamUI>());
interesting... was this perhaps a case where a gUM call might never be called
back?
or would the case of a missing delegate be only in a case of shutting down (and
calling back doesn't matter)?

[Henrik Grunell, 2014-09-22 10:34:52]

Michael, Ilya and Fady, please review.

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
File extensions/browser/guest_view/web_view/web_view_permission_helper.cc
(right):

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:199: if
(!web_view_guest()->embedder_web_contents()->GetDelegate())
On 2014/09/20 14:30:35, tommi wrote:
> is this a fix for another bug?

This function was add a few days ago, and I noticed it should have this check.

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:223:
scoped_ptr<content::MediaStreamUI>());
On 2014/09/20 14:30:35, tommi wrote:
> interesting... was this perhaps a case where a gUM call might never be called
> back?
> or would the case of a missing delegate be only in a case of shutting down
(and
> calling back doesn't matter)?

That's exactly was I was wondering too and mailed fsamuel@ about. I haven't dug
into the order of destruction etc in the shutdown case, but I agree it doesn't
really matter for that case.

fsamuel@: Please comment on this.

[michaelbai, 2014-09-22 18:22:30]

lgtm for android_webview

[Fady Samuel, 2014-09-22 18:27:07]

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
File extensions/browser/guest_view/web_view/web_view_permission_helper.cc
(right):

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:223:
scoped_ptr<content::MediaStreamUI>());
On 2014/09/22 10:34:51, Henrik Grunell wrote:
> On 2014/09/20 14:30:35, tommi wrote:
> > interesting... was this perhaps a case where a gUM call might never be
called
> > back?
> > or would the case of a missing delegate be only in a case of shutting down
> (and
> > calling back doesn't matter)?
> 
> That's exactly was I was wondering too and mailed fsamuel@ about. I haven't
dug
> into the order of destruction etc in the shutdown case, but I agree it doesn't
> really matter for that case.
> 
> fsamuel@: Please comment on this.

A WebViewGuest may not be attached on window.open. In that case, a ne guest is
created but not yet attached to a <webview> element. Resource loads are paused
in this case but for data URLs, code may attempt to access media prior to
attachment. This code would deny it. I'm not sure if
"MEDIA_DEVICE_FAILED_DUE_TO_SHUTDOWN" is the most appropriate error message.

[Ilya Sherman, 2014-09-22 20:56:11]

histograms lgtm

[Henrik Grunell, 2014-09-24 19:19:09]

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
File extensions/browser/guest_view/web_view/web_view_permission_helper.cc
(right):

https://codereview.chromium.org/586223002/diff/1/extensions/browser/guest_vie...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:223:
scoped_ptr<content::MediaStreamUI>());
On 2014/09/22 18:27:06, Fady Samuel wrote:
> On 2014/09/22 10:34:51, Henrik Grunell wrote:
> > On 2014/09/20 14:30:35, tommi wrote:
> > > interesting... was this perhaps a case where a gUM call might never be
> called
> > > back?
> > > or would the case of a missing delegate be only in a case of shutting down
> > (and
> > > calling back doesn't matter)?
> > 
> > That's exactly was I was wondering too and mailed fsamuel@ about. I haven't
> dug
> > into the order of destruction etc in the shutdown case, but I agree it
doesn't
> > really matter for that case.
> > 
> > fsamuel@: Please comment on this.
> 
> A WebViewGuest may not be attached on window.open. In that case, a ne guest is
> created but not yet attached to a <webview> element. Resource loads are paused
> in this case but for data URLs, code may attempt to access media prior to
> attachment. This code would deny it. I'm not sure if
> "MEDIA_DEVICE_FAILED_DUE_TO_SHUTDOWN" is the most appropriate error message.

I interpret this to apply to the "attached" part of the condition. I changed so
that it keeps the "invalid state" result. If no delegate however, I use the new
result.

[Henrik Grunell, 2014-09-24 19:24:05]

After chatting offline, keeping the "invalid state" also for the "no delegate"
case, it doesn't necessarily mean we're shutting down.

[Fady Samuel, 2014-09-24 19:27:03]

https://codereview.chromium.org/586223002/diff/80001/extensions/browser/guest...
File extensions/browser/guest_view/web_view/web_view_permission_helper.cc
(right):

https://codereview.chromium.org/586223002/diff/80001/extensions/browser/guest...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:199: if
(!web_view_guest()->embedder_web_contents()->GetDelegate())
Please check for attached() first (this means we do have an
embedder_web_contents():

if (!web_view_guest()->attached() ||
    !web_view_guest()->embedder_web_contents()->GetDelelgate()) {
  return false;
}

[Henrik Grunell, 2014-09-24 19:40:18]

https://codereview.chromium.org/586223002/diff/80001/extensions/browser/guest...
File extensions/browser/guest_view/web_view/web_view_permission_helper.cc
(right):

https://codereview.chromium.org/586223002/diff/80001/extensions/browser/guest...
extensions/browser/guest_view/web_view/web_view_permission_helper.cc:199: if
(!web_view_guest()->embedder_web_contents()->GetDelegate())
On 2014/09/24 19:27:03, Fady Samuel wrote:
> Please check for attached() first (this means we do have an
> embedder_web_contents():
> 
> if (!web_view_guest()->attached() ||
>     !web_view_guest()->embedder_web_contents()->GetDelelgate()) {
>   return false;
> }

Done.

[Fady Samuel, 2014-09-24 19:43:05]

guest_view lgtm

[Henrik Grunell, 2014-09-24 19:54:28]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-09-24 19:55:26]

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

[commit-bot: I haz the power, 2014-09-24 21:10:18]

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

[commit-bot: I haz the power, 2014-09-24 21:10:20]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Henrik Grunell, 2014-09-25 20:51:41]

The CQ bit was checked by grunell@chromium.org

[commit-bot: I haz the power, 2014-09-25 20:53:24]

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

[commit-bot: I haz the power, 2014-09-25 22:30:16]

Committed patchset #9 (id:160001) as 34c2e7a46a2b7f41df16677dd46a07247385de61

[commit-bot: I haz the power, 2014-09-25 22:31:01]

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