[Media Router] Media Router uses outdated presentation request when the MR UI is kept open |
|||||||||
Issue description1) 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.
,
Jun 3 2016
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.
,
Jun 3 2016
,
Jun 3 2016
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)
,
Jun 7 2016
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.
,
Jun 7 2016
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.
,
Jun 7 2016
The dialog auto-closes after 3s anyways, doesn't it?
,
Jun 7 2016
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.
,
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
,
Jun 10 2016
,
Jun 10 2016
Your change meets the bar and is auto-approved for M52 (branch: 2743)
,
Jun 10 2016
Please have the CL merged to M52 branch so that it gets picked up for Beta Promotion scheduled on 06/15.
,
Jun 11 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
,
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
,
Jun 15 2016
imcheng@, can you please let us know if this can be tested manually so that we can verify it at our end ?
,
Jun 15 2016
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.
,
Jun 27 2016
With 52.0.2743.41, started route and MR dialog auto closed. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by sko...@chromium.org
, Jun 3 2016Status: Assigned (was: Untriaged)