Refactor to make the MediaDecoder focuses on the policy and the MDSM focuses on the mechanism.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=90f4b16baa15e1d28e50d6d04fc823a82c6624f8 https://treeherder.mozilla.org/#/jobs?repo=try&revision=141c2eac99566fc7d30bb42291e9092bd189e711

compile error......., will upload again.

Try again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=00dbaa4ca26276f4503ac911228400533fae58f3 https://treeherder.mozilla.org/#/jobs?repo=try&revision=c61a4290dc0ea5eaa1cd58a4c78dadf055ecd6be

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b9a3548fed58ed70264b75abfbfa2fdc95df84f https://treeherder.mozilla.org/#/jobs?repo=try&revision=41120dcac26033d3581c81cb8adf6343bf1bee96

Comment on attachment 8846274 [details] Bug 1346498 part 9 - move all policy codes into MediaDecoder::UpdateVideoDecodeMode(); https://reviewboard.mozilla.org/r/119380/#review121312 ::: dom/media/MediaDecoder.cpp:399 (Diff revision 1) > , mPinnedForSeek(false) > , mMinimizePreroll(false) > , mMediaTracksConstructed(false) > , mFiredMetadataLoaded(false) > - , mElementVisible(!aOwner->IsHidden()) > + , mIsDocumentVisible(!aOwner->IsHidden()) > + , mIsElementVisible(!aOwner->IsHidden()) Initialize mIsElementVisible to "!aOwner->IsHidden()" is not right, however, it doesn't matter because the mIsElementVisible is not read until the MediaDecoder::NotifyOwnerActivityChanged() is call and MediaDecoder::NotifyOwnerActivityChanged() assigns the right value to mIsElementVisible. This issue will be fixed at bug 1346116 patch 1. https://reviewboard.mozilla.org/r/119452/diff/1#index_header

Comment on attachment 8846256 [details] Bug 1346498 part 5 - don't check mHasSuspendTaint in HandleVideoSuspendTimeout(); https://reviewboard.mozilla.org/r/119368/#review121342

Comment on attachment 8846253 [details] Bug 1346498 part 2 - implement the VideoDecodeMode mechanism in MDSM; https://reviewboard.mozilla.org/r/119362/#review121340 ::: dom/media/MediaDecoder.h:820 (Diff revision 3) > > // True if the decoder has a suspend taint - meaning suspend-video-decoder is > // disabled. > Canonical<bool> mHasSuspendTaint; > > + Canonical<VideoDecodeMode> mVideoDecodeMode; I can't see the point to use Canonical/Mirror here for MediaDecoder never reads mVideoDecodeMode. You can just add a setter to MDSM.

Comment on attachment 8846251 [details] Bug 1346498 part 1 - extract the MediaDecoder::NotifyCompositor() method; https://reviewboard.mozilla.org/r/119358/#review121344

Comment on attachment 8846253 [details] Bug 1346498 part 2 - implement the VideoDecodeMode mechanism in MDSM; https://reviewboard.mozilla.org/r/119362/#review121386 ::: dom/media/MediaDecoderStateMachine.cpp:3078 (Diff revision 4) > +{ > + nsCOMPtr<nsIRunnable> r = > + NewRunnableMethod<VideoDecodeMode>(this, > + &MediaDecoderStateMachine::SetVideoDecodeModeInternal, > + aMode); > + OwnerThread()->Dispatch(r.forget()); Call DispatchStateChange() for this is indeed a state change.

Comment on attachment 8846254 [details] Bug 1346498 part 3 - implement the UpdateVideoDecodeMode() policy in MediaDecoder; https://reviewboard.mozilla.org/r/119364/#review121388

Comment on attachment 8846255 [details] Bug 1346498 part 4 - remove mIsVisible cannonical-mirror pair; https://reviewboard.mozilla.org/r/119366/#review121392

Comment on attachment 8846257 [details] Bug 1346498 part 6 - remove mHasSuspendTaint cannonical-mirror pair; https://reviewboard.mozilla.org/r/119370/#review121394

Comment on attachment 8846272 [details] Bug 1346498 part 7 - remove outdated comments in HTMLMediaElement.cpp; https://reviewboard.mozilla.org/r/119376/#review121396

Comment on attachment 8846273 [details] Bug 1346498 part 8 - extract the HTMLMediaElement::NotifyDecoderActivityChanges() method; https://reviewboard.mozilla.org/r/119378/#review121400 ::: dom/html/HTMLMediaElement.h:1303 (Diff revision 2) > void NotifyAboutPlaying(); > > already_AddRefed<Promise> CreateDOMPromise(ErrorResult& aRv) const; > > + // Pass information for deciding the video decode mode to decoder. > + void SetActiviyChangesToDecoder() const; Might be good to call it "NotifyDecoderActivityChanges".

Comment on attachment 8846274 [details] Bug 1346498 part 9 - move all policy codes into MediaDecoder::UpdateVideoDecodeMode(); https://reviewboard.mozilla.org/r/119380/#review121404 ::: dom/media/MediaDecoder.cpp:1339 (Diff revision 2) > > void > MediaDecoder::UpdateVideoDecodeMode() > { > // The MDSM may yet be set. > if (!mDecoderStateMachine) { If this is called before mDecoderStateMachine is set, we need to propagate the changes to MDSM once it is created.

Comment on attachment 8846274 [details] Bug 1346498 part 9 - move all policy codes into MediaDecoder::UpdateVideoDecodeMode(); https://reviewboard.mozilla.org/r/119380/#review121404 > If this is called before mDecoderStateMachine is set, we need to propagate the changes to MDSM once it is created. Will then call MediaDecoder::UpdateVideoDecodeMode() here: http://searchfox.org/mozilla-central/rev/ef0b6a528e0e7b354ddf8cdce08392be8b8ca679/dom/media/MediaDecoder.cpp#1480

Comment on attachment 8846274 [details] Bug 1346498 part 9 - move all policy codes into MediaDecoder::UpdateVideoDecodeMode(); https://reviewboard.mozilla.org/r/119380/#review121404 > Will then call MediaDecoder::UpdateVideoDecodeMode() here: http://searchfox.org/mozilla-central/rev/ef0b6a528e0e7b354ddf8cdce08392be8b8ca679/dom/media/MediaDecoder.cpp#1480 This issue belongs to patch-3.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2d58ae40ec66d357b9038019b4b0ee541f51cbc1 https://treeherder.mozilla.org/#/jobs?repo=try&revision=4ce13cf27550eabf5e753332ce7e11ac5ed082df

Try looks good, thanks for review!

Pushed by ihsiao@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1152ca4a710e part 1 - extract the MediaDecoder::NotifyCompositor() method; r=jwwang https://hg.mozilla.org/integration/autoland/rev/caaa45f70bb7 part 2 - implement the VideoDecodeMode mechanism in MDSM; r=jwwang https://hg.mozilla.org/integration/autoland/rev/af2fd441e1b2 part 3 - implement the UpdateVideoDecodeMode() policy in MediaDecoder; r=jwwang https://hg.mozilla.org/integration/autoland/rev/a42487bf567a part 4 - remove mIsVisible cannonical-mirror pair; r=jwwang https://hg.mozilla.org/integration/autoland/rev/6cc263c1f298 part 5 - don't check mHasSuspendTaint in HandleVideoSuspendTimeout(); r=jwwang https://hg.mozilla.org/integration/autoland/rev/f4fc110ccde1 part 6 - remove mHasSuspendTaint cannonical-mirror pair; r=jwwang https://hg.mozilla.org/integration/autoland/rev/d450a061b0be part 7 - remove outdated comments in HTMLMediaElement.cpp; r=jwwang https://hg.mozilla.org/integration/autoland/rev/2fbdf43912c4 part 8 - extract the HTMLMediaElement::NotifyDecoderActivityChanges() method; r=jwwang https://hg.mozilla.org/integration/autoland/rev/0f25d813f75d part 9 - move all policy codes into MediaDecoder::UpdateVideoDecodeMode(); r=jwwang

https://hg.mozilla.org/mozilla-central/rev/1152ca4a710e https://hg.mozilla.org/mozilla-central/rev/caaa45f70bb7 https://hg.mozilla.org/mozilla-central/rev/af2fd441e1b2 https://hg.mozilla.org/mozilla-central/rev/a42487bf567a https://hg.mozilla.org/mozilla-central/rev/6cc263c1f298 https://hg.mozilla.org/mozilla-central/rev/f4fc110ccde1 https://hg.mozilla.org/mozilla-central/rev/d450a061b0be https://hg.mozilla.org/mozilla-central/rev/2fbdf43912c4 https://hg.mozilla.org/mozilla-central/rev/0f25d813f75d