[Cast,Android] Subscribe to onCancel and onDismiss correctly

[Cast,Android] Subscribe to onCancel and onDismiss correctly

Subclass DialogFragment classes to get notified about the dialog being cancelled and dismissed.
Don't call dismiss() from the onDismiss() handler.

BaseMediaRouteDialogManager:
- expose mDialogFragment to avoid calling closeDialog() from onDismiss()
- add SystemVisibilitySaver to handle the dialogs showing when in FS mode
  (before was handled by ChromeMediaRouteDialogFactory).

MediaRouteChooserDialogManager:
- use subclassed MediaRouteChooserDialogFragment
- use SystemVisibilitySaver and handle onCancel() and onDismiss()

MediaRouteControllerDialogManager:
- use subclassed MediaRouteControllerDialogFragment
- use SystemVisibilitySaver and handle onDismiss() as onCancel() isn't called.

ChromeMediaRouteDialogController:
- exit onDialogCancelled() early if onRouteClosed() was called before (see the comment).

BUG=551184
Committed: https://crrev.com/68cf0b8360a8ea1dbcd5efb524e7831e1c22bd66
Cr-Commit-Position: refs/heads/master@{#361406}

[whywhat, 2015-11-20 06:16:30]

The CQ bit was checked by avayvod@chromium.org to run a CQ dry run

[whywhat, 2015-11-20 06:16:43]

avayvod@chromium.org changed reviewers:
+ mlamouri@chromium.org

[whywhat, 2015-11-20 06:16:43]

PTaL

[commit-bot: I haz the power, 2015-11-20 06:17:00]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1463693002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1463693002/1

[commit-bot: I haz the power, 2015-11-20 08:34:35]

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

[commit-bot: I haz the power, 2015-11-20 08:34:36]

Dry run: This issue passed the CQ dry run.

[mlamouri (slow - plz ping), 2015-11-24 14:15:40]

lgtm with comments applied.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java
(right):

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java:31:
protected static class SystemVisibilitySaver {
Can you add a TODO to refactor this?

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java:46:
if (mRestoreSystemVisibility) {
if (!mRestoreSystemVisibility) return;

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java:51:
if ((systemVisibility & View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN) != 0) {
nit: you could also do an early return here.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteChooserDialogManager.java
(right):

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteChooserDialogManager.java:43:
mVisibilitySaver.saveSystemVisibility(getActivity());
Why not having this in onStart() to mirror onStop()?

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteControllerDialogManager.java
(right):

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteControllerDialogManager.java:52:
mVisibilitySaver.saveSystemVisibility(getActivity());
ditto

[whywhat, 2015-11-24 18:02:58]

Thanks for the review.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java
(right):

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java:31:
protected static class SystemVisibilitySaver {
On 2015/11/24 at 14:15:40, Mounir Lamouri wrote:
> Can you add a TODO to refactor this?

Done.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java:46:
if (mRestoreSystemVisibility) {
On 2015/11/24 at 14:15:40, Mounir Lamouri wrote:
> if (!mRestoreSystemVisibility) return;

Done.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/BaseMediaRouteDialogManager.java:51:
if ((systemVisibility & View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN) != 0) {
On 2015/11/24 at 14:15:40, Mounir Lamouri wrote:
> nit: you could also do an early return here.

Done.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteChooserDialogManager.java
(right):

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteChooserDialogManager.java:43:
mVisibilitySaver.saveSystemVisibility(getActivity());
On 2015/11/24 at 14:15:40, Mounir Lamouri wrote:
> Why not having this in onStart() to mirror onStop()?

Done.

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteControllerDialogManager.java
(right):

https://codereview.chromium.org/1463693002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/media/router/MediaRouteControllerDialogManager.java:52:
mVisibilitySaver.saveSystemVisibility(getActivity());
On 2015/11/24 at 14:15:40, Mounir Lamouri wrote:
> ditto

Done.

[whywhat, 2015-11-24 18:03:07]

The CQ bit was checked by avayvod@chromium.org

[whywhat, 2015-11-24 18:03:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1463693002/#ps20001
(title: "Addressed the comments.")

[commit-bot: I haz the power, 2015-11-24 18:05:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1463693002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1463693002/20001

[commit-bot: I haz the power, 2015-11-24 18:56:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-24 18:58:41]

Patchset 2 (id:??) landed as
https://crrev.com/68cf0b8360a8ea1dbcd5efb524e7831e1c22bd66
Cr-Commit-Position: refs/heads/master@{#361406}