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

Issue 605165 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

[Media Router] Inconsistent field trial used throughout runtime

Project Member Reported by imch...@chromium.org, Apr 20 2016

Issue description

For users with the Cast Extension, when the component toolbar is being initialized, it will check for MediaRouterEnabled(), which will query the EnableMediaRouterWithCastExtension field trial. If it returned true, the Cast Extension will be unloaded from the extension system. Because of the way MediaRouterEnabled() is implemented, subsequent calls of MediaRouterEnabled() will query the EnableMediaRouter field trial instead, because the Cast Extension is unloaded and no longer in the set of enabled extensions.

This could lead to weird situations such as the Media Router dialog not being rendered with ERR_INVALID_URL, if the user's field trial config is such that {EnableMediaRouterWithCastExtension -> Enabled, EnableMediaRouter -> Disabled}.
 

Comment 1 by mfo...@chromium.org, Apr 20 2016

Cc: -mfo...@chromium.org
Labels: M-51
Owner: mfo...@chromium.org
Status: Assigned (was: Untriaged)
It will be hard to make sense of the field trial results without a fix for this (and the user will perceive it as a bug).

Comment 2 by mfo...@chromium.org, Apr 20 2016

Cc: -imch...@chromium.org mfo...@chromium.org
Owner: imch...@chromium.org
Cc: derat@chromium.org davidri...@chromium.org

Comment 4 by derat@chromium.org, Apr 21 2016

Cc: -derat@chromium.org
Project Member

Comment 5 by bugdroid1@chromium.org, Apr 22 2016

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

commit f735fb21eb9fdc1cade2b8a30217b8c5dc626b90
Author: imcheng <imcheng@chromium.org>
Date: Fri Apr 22 18:30:30 2016

[Media Router] Ensure the same FeatureSwitch is used for lifetime of the
browser.

We noticed that with Cast Extension enabled, the component action
toolbar logic will end up using the EnableMediaRouterWithCastExtension
field trial during the call to MediaRouterEnabled(). If so, the
Cast Extension will be unloaded. Code executed subsequently use
EnableMediaRouter field trial during the call to MediaRouterEnabled().
This could be a problem for users with certain field trial group
configurations and results in inconsistencies (e.g., has Cast button
in toolbar, but opens error page when clicked due to WebUI not found).

The decision is to no longer rely on the EMRWCE field trial. For one,
the mechanism to switch on BrowserContext to return the correct field
trial consistently require a complicated mechanism, and we will still
end up with some corner cases that result in strange behavior. In
addition, having both field trials make analyzing the data more
difficult.

BUG= 605165 

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

Cr-Commit-Position: refs/heads/master@{#389175}

[modify] https://crrev.com/f735fb21eb9fdc1cade2b8a30217b8c5dc626b90/chrome/browser/media/router/media_router_feature.cc

Labels: Merge-Request-51

Comment 7 by tin...@google.com, Apr 25 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 25 2016

Labels: -merge-approved-51 merge-merged-2704
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/06fe70eb19d9727172c03eba2a8cbeb2d13b90ea

commit 06fe70eb19d9727172c03eba2a8cbeb2d13b90ea
Author: Derek Cheng <imcheng@chromium.org>
Date: Mon Apr 25 17:28:27 2016

[Media Router] Ensure the same FeatureSwitch is used for lifetime of the browser.

We noticed that with Cast Extension enabled, the component action
toolbar logic will end up using the EnableMediaRouterWithCastExtension
field trial during the call to MediaRouterEnabled(). If so, the
Cast Extension will be unloaded. Code executed subsequently use
EnableMediaRouter field trial during the call to MediaRouterEnabled().
This could be a problem for users with certain field trial group
configurations and results in inconsistencies (e.g., has Cast button
in toolbar, but opens error page when clicked due to WebUI not found).

The decision is to no longer rely on the EMRWCE field trial. For one,
the mechanism to switch on BrowserContext to return the correct field
trial consistently require a complicated mechanism, and we will still
end up with some corner cases that result in strange behavior. In
addition, having both field trials make analyzing the data more
difficult.

BUG= 605165 

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

Cr-Commit-Position: refs/heads/master@{#389175}
(cherry picked from commit f735fb21eb9fdc1cade2b8a30217b8c5dc626b90)

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

Cr-Commit-Position: refs/branch-heads/2704@{#220}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/06fe70eb19d9727172c03eba2a8cbeb2d13b90ea/chrome/browser/media/router/media_router_feature.cc

Status: Fixed (was: Assigned)
I'm seeing what looks like this failure on ChromeOS (making chromecast unusable):
  Google Chrome	50.0.2661.91 (Official Build) beta (64-bit)

I tried logging out of my corp account, rebooting, and logging back in with my personal account (to get different field trials), but that had the same issue.

This this make it to M50 stable?  Something wrong with the finch config perhaps?

I submitted feedback but it hasn't shown up on the feedback site yet.
Screenshot 2016-05-02 at 9.10.05 PM.png
48.8 KB View Download
The fix wasn't merged back to M50. It will be fixed once it is updated to M51. I'm not sure why it M51 hasn't been pushed to beta ChromeOS yet though. For now, you can work around it by setting chrome://flags#media-router to Enabled. Sorry for the inconvenience. 
Cc: imch...@chromium.org
 Issue 608602  has been merged into this issue.

Sign in to add a comment