New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 617321 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jun 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

[Media Router] Media Router uses outdated presentation request when the MR UI is kept open

Project Member Reported by vadimgo@chromium.org, Jun 3 2016

Issue description

1) Create a page to call start on a PresentationRequest, but do not set defaultRequest.
2) Go to MR UI and select a device to cast to.
3) After the request is resolved, with the MR UI still open, stop the route, and then click on the device again to cast again.

MR extension gets a call to create a route using the presentation request from step 1. But since that request is already resolved and there is no default request, the app is in a confused state.

MR UI should clear the context of the presentation request, after it's resolved or rejected and not try to re-use it.
 
Labels: M-53
Status: Assigned (was: Untriaged)
Is this a recent regression or something that's been broken for a while?
I don't think it's a regression, but the use of the stale presentation URL puts MR extension and the MRP into a bad state, resulting in a very bad experience for the end user.
Cc: sko...@chromium.org vadimgo@chromium.org
It's been like that for a while. The current logic of the UI is such that presentation URLs from the request sticks around in the UI even after the request has been resolved/rejected. It allows the user to cast to the presentation again (i.e step 3 in the repro steps) but the UI has already been detached from the request. (since the request can be responded to only once)
This scenario is not exactly defined from a spec point of view, since the Promise returned by start() must be settled once and exactly once.  Perhaps we should just close the dialog after resolving start()?

Changing the presentation URL while the dialog is open is also something we may not handle in a well defined way, yet.
Re #5: closing the dialog will be simpler so I am in favor of it. The user will have to reopen the dialog to see the route details but that's probably ok.
The dialog auto-closes after 3s anyways, doesn't it?  
Cc: mfo...@chromium.org
Re #7. The bug happens if the dialog is kept alive by moving the mouse over it, in which case it won't autoclose.

Re #5.  Closing the dialog sounds good to me and should be most reasonable for the end user, especially while defaultRequest from an iframe does not work as expected.

Otherwise, if staying in the dialog, it should not use a stale presentation request for which start was already resolved - it should use defaultRequest instead and if defaultRequest is not set, it should mirror.  But again, it may just be best to close the dialog.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 8 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbe9df4959c78e3d8a4fbaa1fef5ccde9b4c0f3f

commit cbe9df4959c78e3d8a4fbaa1fef5ccde9b4c0f3f
Author: btolsch <btolsch@chromium.org>
Date: Wed Jun 08 21:30:58 2016

[Media Router] Close dialog after resolving presentation request

This change causes the Media Router dialog to close after it resolves a
route that is from a presentation request. This prevents the dialog from
staying open (by the user keeping their mouse over it) and reusing the
same presentation request for another route request (which is an error).

BUG= 617321 
R=imcheng@chromium.org

Review-Url: https://codereview.chromium.org/2038423006
Cr-Commit-Position: refs/heads/master@{#398683}

[modify] https://crrev.com/cbe9df4959c78e3d8a4fbaa1fef5ccde9b4c0f3f/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/cbe9df4959c78e3d8a4fbaa1fef5ccde9b4c0f3f/chrome/browser/ui/webui/media_router/media_router_ui.h

Labels: Merge-Request-52

Comment 11 by tin...@google.com, Jun 10 2016

Labels: -Merge-Request-52 Merge-Approved-52 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M52 (branch: 2743)
Please have the CL merged to M52 branch so that it gets picked up for Beta Promotion scheduled on 06/15.
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 11 2016

Labels: -merge-approved-52 merge-merged-2743
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/827411a04371fcaefd879fbc7e13061a232f1cea

commit 827411a04371fcaefd879fbc7e13061a232f1cea
Author: Derek Cheng <imcheng@chromium.org>
Date: Sat Jun 11 16:44:01 2016

[Media Router] Close dialog after resolving presentation request

This change causes the Media Router dialog to close after it resolves a
route that is from a presentation request. This prevents the dialog from
staying open (by the user keeping their mouse over it) and reusing the
same presentation request for another route request (which is an error).

BUG= 617321 
R=imcheng@chromium.org

Review-Url: https://codereview.chromium.org/2038423006
Cr-Commit-Position: refs/heads/master@{#398683}
(cherry picked from commit cbe9df4959c78e3d8a4fbaa1fef5ccde9b4c0f3f)

Review URL: https://codereview.chromium.org/2060433004 .

Cr-Commit-Position: refs/branch-heads/2743@{#327}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/827411a04371fcaefd879fbc7e13061a232f1cea/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/827411a04371fcaefd879fbc7e13061a232f1cea/chrome/browser/ui/webui/media_router/media_router_ui.h

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/827411a04371fcaefd879fbc7e13061a232f1cea

commit 827411a04371fcaefd879fbc7e13061a232f1cea
Author: Derek Cheng <imcheng@chromium.org>
Date: Sat Jun 11 16:44:01 2016

[Media Router] Close dialog after resolving presentation request

This change causes the Media Router dialog to close after it resolves a
route that is from a presentation request. This prevents the dialog from
staying open (by the user keeping their mouse over it) and reusing the
same presentation request for another route request (which is an error).

BUG= 617321 
R=imcheng@chromium.org

Review-Url: https://codereview.chromium.org/2038423006
Cr-Commit-Position: refs/heads/master@{#398683}
(cherry picked from commit cbe9df4959c78e3d8a4fbaa1fef5ccde9b4c0f3f)

Review URL: https://codereview.chromium.org/2060433004 .

Cr-Commit-Position: refs/branch-heads/2743@{#327}
Cr-Branched-From: 2b3ae3b8090361f8af5a611712fc1a5ab2de53cb-refs/heads/master@{#394939}

[modify] https://crrev.com/827411a04371fcaefd879fbc7e13061a232f1cea/chrome/browser/ui/webui/media_router/media_router_ui.cc
[modify] https://crrev.com/827411a04371fcaefd879fbc7e13061a232f1cea/chrome/browser/ui/webui/media_router/media_router_ui.h

Labels: Needs-Feedback
imcheng@, can you please let us know if this can be tested manually so that we can verify it at our end ?
Cc: imch...@chromium.org
Owner: dbbrooks@chromium.org
dbbrooks@, can you help us verify the fix in 52.0.2743.41? If you open the MR dialog from a cast icon on the page and start a route there, then the dialog should be closed automatically after it succeeds or fails.
Status: Verified (was: Assigned)
With 52.0.2743.41, started route and MR dialog auto closed.

Sign in to add a comment