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

Issue 882690 link

Starred by 1 user
Status: Fixed
Owner:
█ imch...@chromium.org
Last visit > 30 days ago
Closed: Sep 24
Cc:
mfo...@chromium.org
btolsch@chromium.org
powerb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[Presentation API] Close / Remove PresentationConnections on navigation

Project Member Reported by imch...@chromium.org, Sep 11 
Currently, Media Route Providers are informed of PresentationConnections closing via PresentationServiceImpl observing navigation and issuing MediaRouter::DetachRoute() calls to MediaRouter. As we replace PresentationService API calls with PresentationConnection, it would be better to do the following:

- Move the MediaRouter::DetachRoute() that's made from PresentationServiceImpl::CloseConnection() to BrowserPresentationConnectionProxy::RequestClose(). Note that Blink currently calls both RequestClose() and CloseConnection(). This takes care of the case when the frame calls close() on the presentation connection.

- PresentationController to tear down / remove PresentationConnections during ContextDestroyed(). This should take care of the navigation case, and also makes sure resources are cleaned up.

- On the native Cast MRP side, we will implement RequestClose() to close the Mojo pipes and remove the corresponding client from the CastActivityRecord.

Comment 1 by powerb@chromium.org, Sep 11

Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 19 

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

commit 2c99b53dc248ac4c1f50d084ef9d8779bb491862
Author: Derek Cheng <imcheng@chromium.org>
Date: Wed Sep 19 21:36:50 2018

[Presentation API] Close PresentationConnection on contextDestroyed().

By calling close() during contextDestroyed(), the PresentationConnection
in Blink will notify the other end of the mojo message pipe via
RequestClose(). Native MRPs holding the other end of the pipe
(e.g. Cast MRP 2-UA mode) can then use this as a signal to clean up the
presentation, instead of going through MediaRouteProvider::DetachRoute()

This also fixes a bug where in 1-UA mode, the receiver connection does
not receive state change to CLOSED when the controller frame navigates
away.

Note that PresentationService::CloseConnection() is still being called
from Blink. This is still required for 1-UA mode where the other end of
the pipe is not in the browser.

Also, PresentationServiceImpl still performs a Reset() on frame
navigation to clean up other internal states.

Bug:  882690 
Change-Id: I9b4ecb862c39c737b36b2d5435286fb59a3ff752
Reviewed-on: https://chromium-review.googlesource.com/1222679
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592552}
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/android/router/media_router_android.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/android/router/media_router_android.h
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/router/presentation/browser_presentation_connection_proxy.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/router/presentation/browser_presentation_connection_proxy.h
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/router/providers/cast/cast_activity_manager.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/router/providers/cast/cast_activity_manager.h
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/router/providers/cast/cast_activity_manager_unittest.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/media/router/test/test_helper.h
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/chrome/browser/ui/media_router/presentation_receiver_window_controller_browsertest.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/content/browser/presentation/presentation_service_impl_unittest.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/WebKit/LayoutTests/presentation/presentation-controller-close-connection.html
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/WebKit/LayoutTests/presentation/resources/presentation-receiver-close-connection.html
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/blink/public/mojom/presentation/presentation.mojom
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/blink/renderer/modules/presentation/presentation_connection.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/blink/renderer/modules/presentation/presentation_connection.h
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/blink/renderer/modules/remoteplayback/remote_playback.cc
[modify] https://crrev.com/2c99b53dc248ac4c1f50d084ef9d8779bb491862/third_party/blink/renderer/modules/remoteplayback/remote_playback.h

Comment 3 by imch...@chromium.org, Sep 24

Status: Fixed (was: Assigned)
►

Sign in to add a comment