Turn webspeech on/off when tab goes fore/background

Turn webspeech on/off when tab goes fore/background

This change adds a toggle to turn webspeech off when tab goes background.
And further request to turn media capture devices on will be denied.
So Both WebRTC and webSpeech won't be able to initiate new requests when tab is in background.

This CL also fixed a bug in the original CL that was reverted: some times recognition will not be aborted immediately after screen lock.
The issue is caused by that calling abort() will not delete the session in SpeechRecognitionManagerImpl.
So if a SpeechRecognition objects calls multiple start(), several sessions with the same request_id will be created.
And when passing the request_id to abort the sessions, only 1 session will be aborted
This change requests the all sessions in the same render view to be aborted.
Thus solving the above issue.

BUG=396054
R=jochen@chromium.org, tsepez@chromium.org, xians@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285314

[qinmin, 2014-07-24 02:57:55]

Reviewers, please prioritize this code review. See more detail in the crbug.
Thanks

[Tom Sepez, 2014-07-24 03:11:33]

Messages LGTM.

https://codereview.chromium.org/415933002/diff/1/content/common/speech_recogn...
File content/common/speech_recognition_messages.h (right):

https://codereview.chromium.org/415933002/diff/1/content/common/speech_recogn...
content/common/speech_recognition_messages.h:83:
IPC_MESSAGE_CONTROL1(SpeechRecognitionHostMsg_AbortAllRequest,
nit: _AbortAllRequests (plural throughout all uses).

[qinmin, 2014-07-24 03:15:59]

https://codereview.chromium.org/415933002/diff/1/content/common/speech_recogn...
File content/common/speech_recognition_messages.h (right):

https://codereview.chromium.org/415933002/diff/1/content/common/speech_recogn...
content/common/speech_recognition_messages.h:83:
IPC_MESSAGE_CONTROL1(SpeechRecognitionHostMsg_AbortAllRequest,
On 2014/07/24 03:11:33, Tom Sepez wrote:
> nit: _AbortAllRequests (plural throughout all uses).

Done.

[jochen (gone - plz use gerrit), 2014-07-24 08:49:54]

please add a test for this, separate CL is ok if you want to merge this

code changes lgtm

[no longer working on chromium, 2014-07-24 12:36:45]

Hi Min,

I took a look at the speech code together with burnik, but it is a bit hard for
me to understand the problems.

From SpeechRecognition.cpp, a webkit SpeechRecognition object will throw an
exception if users call start() after the recognition has been started, it is
done by keeping track of m_started flag, and m_started will be reset to false
only after getting an didEnd() or didReceiveError() methods.

And in speech_recognition_manager_impl.cc, we delete the session when getting a
OnRecognitionEnd() callback, which happens before passing didEnd() event to
blink.

From what I observe so far, it is a bit hard to understand how we can have
multiple sessions for one SpeechRecognition object (identified as request_id on
the dispatcher) at a time. Of course some rare race might still be possible
since the thread model is quite complicated.

If this fix is not urgent to be landed, I would like to put some time to
understand the real problems before injecting code. Could you please provide the
repro steps or some JS code to help us understanding the problems.

[qinmin, 2014-07-24 13:52:37]

Hi, xians, here is the observed problem:
Take a look at the Javascript on
https://www.standardabweichung.de/design/projekte/html5/chrome-android-mic-se...

1. when locking the screen, a didError() and a didEnd() will be sent to the
SpeechRecognition object as the result of abortAllRecognitions().
2. On the test page, didError() and didEnd() will trigger 2 Start() calls,
however, only one of them will get through. I think the other one gets a
Dom Exception
3. Start() will be rejected by the new code as device controllers are not
allowed to be used, so an onError will be sent to SpeechRecognitioj
4. OnError will trigger another Start, thus causing an infinite loop
between DidError/Start loop.
5. However, SpeechRecognitionDispatcher does not always create a request_id
when start is called, sometimes it reuses the old id.
6. SpeechRecognitionManagerImpl, on the otherhand, always create a new
session_id
7. when screen unlocks, we tried to abort all request_id inside
SpeechRecognitionDispatcher,
and since the number of request_id<session_id, only a small number of them
is actually aborted

The issue seems that session_id can only be deleted when RecognitionEnd
message is received while it is in an SpeechRecognitionManagerImpl Idle
state. However, I think that was not happening.

On Thu, Jul 24, 2014 at 5:36 AM, <xians@chromium.org> wrote:

>
> Hi Min,
>
> I took a look at the speech code together with burnik, but it is a bit
> hard for
> me to understand the problems.
>
> From SpeechRecognition.cpp, a webkit SpeechRecognition object will throw an
> exception if users call start() after the recognition has been started, it
> is
> done by keeping track of m_started flag, and m_started will be reset to
> false
> only after getting an didEnd() or didReceiveError() methods.
>
> And in speech_recognition_manager_impl.cc, we delete the session when
> getting a
> OnRecognitionEnd() callback, which happens before passing didEnd() event to
> blink.
>
> From what I observe so far, it is a bit hard to understand how we can have
> multiple sessions for one SpeechRecognition object (identified as
> request_id on
> the dispatcher) at a time. Of course some rare race might still be possible
> since the thread model is quite complicated.
>
> If this fix is not urgent to be landed, I would like to put some time to
> understand the real problems before injecting code. Could you please
> provide the
> repro steps or some JS code to help us understanding the problems.
>
>
> https://codereview.chromium.org/415933002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[qinmin, 2014-07-24 14:45:37]

ok, merged the previous CL into this CL as it is reverted

[no longer working on chromium, 2014-07-24 15:18:23]

On 2014/07/24 14:45:37, qinmin wrote:
> ok, merged the previous CL into this CL as it is reverted

Kristijan tried playing around the speech JS code but could not reproduce the
problems on Linux, it might be some rare racing that is hard to reproduce on
Linux.

I discussed with Min offline, we decided to create new issue to track the
request id -> multiple session id problem, so we can fix the privacy issues
ASAP.

lgtm

[no longer working on chromium, 2014-07-24 15:19:08]

The CQ bit was checked by xians@chromium.org

[commit-bot: I haz the power, 2014-07-24 15:20:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/qinmin@chromium.org/415933002/40001

[qinmin, 2014-07-24 17:42:01]

Committed patchset #3 manually as r285314 (presubmit successful).