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

Issue 861778 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jul 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[Media Router] MR extension crashes while attempting to cast, then extension no longer works

Project Member Reported by imch...@chromium.org, Jul 9

Issue description

This is spin off from internal bug where attempting to cast to Cast EDU causes an indefinite spinner, and extension-related functionalities no longer work (e.g. no devices found) afterwards. This is not a new regression, but rather have existed for a while and is a somewhat uncommon occurrence. We will track the cause of extension crash separately.

(1) the extension is suspected to crash during the request to cast. This causes the Mojo message pipes between the extension and ExtensionMediaRouteProviderProxy to error out, and the CreateRoute request is unfulfilled (i.e. callback supplied to extension was dropped).
(2) ExtensionMediaRouteProviderProxy itself implements the MediaRouteProvider interface. MediaRouter calls CreateRoute on it, and it calls CreateRoute on the MR extension, passing through the supplied callback. This means when (1) happens, the MediaRouter's CreateRoute callback is dropped as well. This results in a Mojo connection error between MediaRouter and EMRPP and is currently unrecoverable.
(3) Note it is possible to reach this bad state via other means, such as if a queued request with a Mojo callback gets dropped.

We could fix this in one of following ways:
(1) add a layer of indirection in EMRPP for all MediaRouteProvider methods that has a callback by wrapping the callbacks and tracking them. When an extension error is detected, invoke the pending callbacks with failure values. This ensures EMRPP always answers callbacks regardless of extension state.
(2) let the mojo connection error propagate back to MediaRouter. Let MediaRouter detects this case and recreate the Mojo message pipe to ExtensionMediaRouteProviderProxy. Cap this to a maximum number of attempts and send issue. Also record to metrics when this happens.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 12

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

commit da7bd5a1bbf177bb5822564193a7ea1795e0159b
Author: Derek Cheng <imcheng@chromium.org>
Date: Thu Jul 12 03:34:13 2018

[MediaRouter] Reconnect the extension MRP Mojo pipe on connection error.

MR extension can fail/crash for many reasons. This in turn may cause
ExtensionMediaRouteProviderProxy to violate its Mojo message pipe
contract by dropping pending callbacks.

Rather than wrapping each callback and tracking them in EMRPP, this
patch deals with this by having MediaRouterDesktop detect when the EMRPP
pipe encounters an error and attempt to recreate the pipe, subject to a
max number of attempts.

Bug:  861778 
Change-Id: Ie70e2cae3ea6650b65f658aba4ec527d4e2eb0d0
Reviewed-on: https://chromium-review.googlesource.com/1129208
Reviewed-by: Takumi Fujimoto <takumif@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574479}
[modify] https://crrev.com/da7bd5a1bbf177bb5822564193a7ea1795e0159b/chrome/browser/media/router/mojo/media_router_desktop.cc
[modify] https://crrev.com/da7bd5a1bbf177bb5822564193a7ea1795e0159b/chrome/browser/media/router/mojo/media_router_desktop.h
[modify] https://crrev.com/da7bd5a1bbf177bb5822564193a7ea1795e0159b/chrome/browser/media/router/mojo/media_router_desktop_unittest.cc
[modify] https://crrev.com/da7bd5a1bbf177bb5822564193a7ea1795e0159b/chrome/browser/media/router/providers/extension/extension_media_route_provider_proxy.cc
[modify] https://crrev.com/da7bd5a1bbf177bb5822564193a7ea1795e0159b/chrome/browser/media/router/providers/extension/extension_media_route_provider_proxy.h
[modify] https://crrev.com/da7bd5a1bbf177bb5822564193a7ea1795e0159b/chrome/browser/media/router/providers/extension/extension_media_route_provider_proxy_unittest.cc

Cc: imch...@chromium.org
Status: Verified (was: Started)
I haven't been able to repro the bad state since the fix landed, so I am marking it as fixed.

Sign in to add a comment