Move the code of MDSM::SetDormant to various state objects

RESOLVED FIXED in Firefox 52

Status

()

defect
P3
normal
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jwwang, Assigned: jwwang)

Tracking

unspecified
mozilla52
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox52 fixed)

Details

Attachments

(4 attachments)

Assignee

Description

3 years ago
No description provided.
Assignee

Updated

3 years ago
Assignee: nobody → jwwang
Blocks: 1295892
Priority: -- → P3
Assignee

Updated

3 years ago
Attachment #8792368 - Flags: review?(kikuo)
Attachment #8792369 - Flags: review?(kikuo)
Attachment #8792370 - Flags: review?(kikuo)
Attachment #8792371 - Flags: review?(kikuo)

Comment 5

3 years ago
mozreview-review
Comment on attachment 8792369 [details]
Bug 1302006. Part 2 - Let ShutdownState handle dormant request.

https://reviewboard.mozilla.org/r/79428/#review78038
Attachment #8792369 - Flags: review?(kikuo) → review+

Comment 6

3 years ago
mozreview-review
Comment on attachment 8792370 [details]
Bug 1302006. Part 3 - Let SeekingState handle dormant request.

https://reviewboard.mozilla.org/r/79430/#review78048
Attachment #8792370 - Flags: review?(kikuo) → review+

Comment 7

3 years ago
mozreview-review
Comment on attachment 8792368 [details]
Bug 1302006. Part 1 - Let DormantState handle dormant request.

https://reviewboard.mozilla.org/r/79426/#review78052
Attachment #8792368 - Flags: review?(kikuo) → review+

Comment 8

3 years ago
mozreview-review
Comment on attachment 8792371 [details]
Bug 1302006. Part 4 - Let StateObject handle dormant request.

https://reviewboard.mozilla.org/r/79432/#review78066

::: dom/media/MediaDecoderStateMachine.cpp:231
(Diff revision 1)
> -  virtual bool HandleDormant(bool aDormant) { return false; }
> +  virtual bool HandleDormant(bool aDormant)
> +  {
> +    if (!aDormant) {
> +      return true;
> +    }
> +    SLOG("Enter dormant state");

I don't think we need this message here. This information will be shown by this log  "DECODER_LOG("MDSM state: %s -> %s", ToStateStr(), ToStateStr(aState));" in |SetState|.

Unless you want to identify which StateObject is handling dormant, if so, we should add smiliar messages in other StateObject's HandleDormant, i.e.  SeekingState in Part3.
Assignee

Comment 9

3 years ago
mozreview-review
Comment on attachment 8792371 [details]
Bug 1302006. Part 4 - Let StateObject handle dormant request.

https://reviewboard.mozilla.org/r/79432/#review78282

::: dom/media/MediaDecoderStateMachine.cpp:231
(Diff revision 1)
> -  virtual bool HandleDormant(bool aDormant) { return false; }
> +  virtual bool HandleDormant(bool aDormant)
> +  {
> +    if (!aDormant) {
> +      return true;
> +    }
> +    SLOG("Enter dormant state");

Will remove this log.
Comment hidden (mozreview-request)

Comment 11

3 years ago
mozreview-review
Comment on attachment 8792371 [details]
Bug 1302006. Part 4 - Let StateObject handle dormant request.

https://reviewboard.mozilla.org/r/79432/#review78290

Cool work !
Attachment #8792371 - Flags: review?(kikuo) → review+
Assignee

Comment 12

3 years ago
Thanks for the review!

Comment 13

3 years ago
Pushed by jwwang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7fb80e9897fa
Part 1 - Let DormantState handle dormant request. r=kikuo
https://hg.mozilla.org/integration/autoland/rev/66359c23a6b6
Part 2 - Let ShutdownState handle dormant request. r=kikuo
https://hg.mozilla.org/integration/autoland/rev/c12f64bf2598
Part 3 - Let SeekingState handle dormant request. r=kikuo
https://hg.mozilla.org/integration/autoland/rev/e1bf663363fd
Part 4 - Let StateObject handle dormant request. r=kikuo
You need to log in before you can comment on or make changes to this bug.