This is to address the issue found in bug 1193394 comment 44. The "Stable" state is used to run some task not visible from the script. However, there are 2 use cases that breaks this rule: 1. HTMLMediaElement::RunInStableState() -> MediaDecoder::BackgroundVideoDecodingPermissionObserver::EnableEvent(): https://treeherder.mozilla.org/#/jobs?repo=try&revision=1bee6bf1006e7627557053e94dfca13f4754b0b5&selectedJob=137781048 2. MediaStreamGraphImpl::RunInStableState() -> SpeechSynthesisUtterance::DispatchSpeechSynthesisEvent(): https://public-artifacts.taskcluster.net/MimKd4uhQfmBv6NHDVS3Mg/0/public/logs/live_backing.log We should eliminate these abuse problem and add assertion to identify this problem in the future.

1. Add assertion to prevent dispatching non-chrome event in stable states. 2. Use AsyncEventDispatcher in stable states.

Treeherder result looks good, btw: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f10d6d561f1cbb256de309f744afab324c14bdd1

Comment on attachment 8920454 [details] [diff] [review] (v1) Patch: Bug 1409985 - Don't fire DOMEvent during StableState. Review of attachment 8920454 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/events/EventDispatcher.cpp @@ +604,5 @@ > > + // Events shall not be fired to content while we are in stable state. > + MOZ_ASSERT_IF(!aEvent->mFlags.mOnlyChromeDispatch, > + CycleCollectedJSContext::Get() && > + !CycleCollectedJSContext::Get()->IsDoingStableStates()); I should add nsContentUtils::IsDoingStableStates() to simplify the call of MOZ_ASSERT(CycleCollectedJSContext::Get(), "Must be on a script thread!"); MOZ_ASSERT(CycleCollectedJSContext::Get()->IsDoingStableStates());

Why non-chrome event? All the events should be prevented.

(In reply to Olli Pettay [:smaug] (work week Oct 16-20) from comment #4) > Why non-chrome event? All the events should be prevented. I thought it's fine for chrome event per offline discussion and to keep HTMLMediaElement unchanged. I could try AsyncEventDispatcher for HTMLMediaElement to see if there is anything wrong.

Comment on attachment 8920454 [details] [diff] [review] (v1) Patch: Bug 1409985 - Don't fire DOMEvent during StableState. cancel the review per comment 5.

No JS should run during stable state, IMO. Well, possibly some JS implemented XPCOM components.

1. Dispatch async task for the following events which could be fired during stable state: a. MediaDecoder::BackgroundVideoDecodingPermissionObserver::EnableEvent() b. MediaDecoder::BackgroundVideoDecodingPermissionObserver::DisableEvent() c. SpeechSynthesisUtterance::DispatchSpeechSynthesisEvent() 2. Add assertion in EventDispatcher to catch the abuse of RunInStableState(). Treeherder result looks fine, btw: https://treeherder.mozilla.org/#/jobs?repo=try&revision=38d79c55603ae46d3b9b930e5657b7b8a8f309e6&group_state=expanded

Comment on attachment 8921379 [details] [diff] [review] (v2) Patch: Don't fire DOMEvent during StableState. ># HG changeset patch ># User Bevis Tseng <btseng@mozilla.com> ># Date 1508408431 -28800 ># Thu Oct 19 18:20:31 2017 +0800 ># Node ID 36713cb4d1b4e70e8015780c07cbc88520cf7ce7 ># Parent 9056f2ee492fa481aa86146aba236c074628e9fd >Bug 1409985 - Don't fire DOMEvent during StableState. r=smaug > >diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp >--- a/dom/base/nsContentUtils.cpp >+++ b/dom/base/nsContentUtils.cpp >@@ -5831,6 +5831,14 @@ nsContentUtils::RunInMetastableState(alr > } > > /* static */ >+bool >+nsContentUtils::IsDoingStableStates() Could it be called IsInStableOrMetaStableState(), here and elsewhere. >+ if (nsCOMPtr<nsIDocument> doc = GetOwnerDoc()) { >+ doc->Dispatch(TaskCategory::Other, >+ NewRunnableMethod( >+ "MediaDecoder::BackgroundVideoDecodingPermissionObserver::EnableEvent", >+ this, &MediaDecoder::BackgroundVideoDecodingPermissionObserver::EnableEvent)); Nit, use 2 spaces for indentation >+ if (nsCOMPtr<nsIDocument> doc = GetOwnerDoc()) { >+ doc->Dispatch(TaskCategory::Other, >+ NewRunnableMethod( >+ "MediaDecoder::BackgroundVideoDecodingPermissionObserver::DisableEvent", >+ this, &MediaDecoder::BackgroundVideoDecodingPermissionObserver::DisableEvent)); Nit, 2 spaces for indentation >@@ -174,7 +176,16 @@ SpeechSynthesisUtterance::DispatchSpeech > > RefPtr<SpeechSynthesisEvent> event = > SpeechSynthesisEvent::Constructor(this, aEventType, init); >- DispatchTrustedEvent(event); >+ >+ if (nsContentUtils::IsDoingStableStates()) { >+ // Events shall not be fired synchronously to prevent anything visible >+ // from the scripts while we are in stable state. >+ RefPtr<AsyncEventDispatcher> asyncDispatcher = >+ new AsyncEventDispatcher(this, event); >+ asyncDispatcher->PostDOMEvent(); >+ } else { >+ DispatchTrustedEvent(event); >+ } Could you explain why this approach, and why not make the MediaStreamcallback thingie to call speech API asynchronously? (I thought we chatted about that on IRC) I think I'd prefer that. But if there is some reason for this approach, explain and ask review again.

1. Rename IsDoingStableStates() to IsInStableOrMetaStableState() 2. Call SynthStreamListener::DoNotify{Started|Ended} asynchronously after stream state update. (In previous patch, besides event dispatching, I thought that there are other stuff in DoNotifyStarted/Ended was expected to be done during stable state, so I only move the event dispatching part to a new task. However, after testing with this latest patch to async call DoNotifyStarted/Ended entirely, everything looks fine on try [1] as well, so I update this one for the review instead.) [1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=1ce4420bbdd4326a7362e7389f93392bba76e086

Pushed by btseng@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/22003af130ed Don't fire DOMEvent during StableState. r=smaug