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

Issue 738677 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

Chrome is crashing when casting second video to Chromecast

Reported by nekr.fab...@gmail.com, Jul 1 2017

Issue description

Steps to reproduce the problem:
1. Open https://omroep-pwa-lugggpwdhh.now.sh/
2. Go to an article with a video (with a "play" icon)
3. Start playing video, then cast it to Chromecast
4. Wait till video starts casting and navigate back to the homepage
5. Go to another article with a video
6. Start playing video and then cast it to Chromecast

What is the expected behavior?
Second video start casting, Chrome isn't crashing

What went wrong?
Chrome crashed

Crashed report ID: crash/6e7d5ee938000000

How much crashed? Just one tab

Is it a problem with a plugin? No 

Did this work before? N/A 

Chrome version:   Channel: stable
OS Version: 
Flash Version: 

I actually have no idea what's going on. At first I thought it's memory leak but I remove at the listeners from old MediaElement and its RemotePlayback object.
 
It seems to be related to this issue: https://bugs.chromium.org/p/chromium/issues/detail?id=737744

i.e. only reproducible after moving DOM element around. If you will try playing video here https://omroep-pwa-lugggpwdhh.now.sh/news/3469548 and then swiping that video in the carousel and playing the next one -- crash won't happen. But if you repeat the steps mentioned in Comment 1, then you'll get a crash.
I found a workaround: if the old media element (which already casts the media) is destroyed prior the next one is even created then Chrome do not crash and normally casts the second video and so on.

If you do not destroy previous casting media element or destroy it later after creating a new one, Chrome crashes consistently all the time.

destroy:
elem.remove(); elem.src = ''; elem.load();
Components: Internals>Cast>MediaFling
Owner: avayvod@chromium.org
avayvod@, can you PTAL?
The problems seems to be in MediaElement#remote event listeners, i.e. 'connect', 'connecting', 'disconnect'.

Another steps what cause the crash:

* Play media to a Chromecast
* Move MediaElement in the DOM
* Stop Chromecast casting from Android's notification
* Chrome crashes

With this steps it doesn't crash:

* Play media to a Chromecast
* Move MediaElement in the DOM
* ---- remove MediaElement#remote event listeners ---
* Stop Chromecast casting from Android's notification
* Chrome crashes

So even though that's a workaround, it's pretty useless because if I use remote casting then I need to listen to its events.

Also, can this be raise in priority or at least looked at it? It constantly causes crashes in a Progressive Web App which I set to my client yesterday to deploy to production. It literally destroys whole remote casting experience.
Labels: -Pri-2 M-60 ReleaseBlock-Stable Pri-1
Sorry to say that, but the earliest the fix will be able to make it to stable atm is Chrome 60 scheduled to be released late July. If you need Chrome not to crash before that, you'd have to workaround somehow.

Also, July 1-4 was a long weekend in the US. I will try to take a look this or at least next week.
I just found that (after writing that message) that using setInterval() and pulling MediaElement#remote.state property instead of listening to connect/connecting/disconnect events doesn't crash Chrome. I think this is the only good enough workaround.

Thanks for replying and raising the priority! (and yeah, I understand the holidays)
Ok. The original link doesn't work for me (no articles to play/cast videos from).
I tried to reproduce the crash with the following steps on the test page:
1. go to http://beaufortfrancois.github.io/sandbox/media/remote-playback.html
2. open dev tools console
3. execute var v = document.querySelector('video');
4. cast a video to a Chromecast
5. execute v.parentNode.removeChild(v); to remove the video from the DOM

Observe video on Chromecast stopping like in  Issue 737744 .

5.1 (optional) execute document.body.appendChild(v);

6. stop casting the video from the notification

I observe no crash and everything except for pausing works as expected. Perhaps the element needs to be attached to a different document.

I also tried using another news site with multiple videos (nytimes.com) and repeating the sequence:
1. go to nytimes.com/videos
2. cast the first video
3. go back, use the menu to get to nytimes.com/videos/art
4. cast a second video

No crash there.
I'm pretty sure I wasn't making that up, otherwise I wouldn't need to working around that. Unfortunately the repro link died indeed and it probably won't be recovered. The project is finished and if I ever go back to it I'll try to deploy an older revision for repro. I guess there should be a bot saving repro pages so they could be used later by the browser devs.

There is also a crash report attached in the first comment.
Cc: sande...@chromium.org
Thanks for pointing out the crash report. +Dan, since it's in WMPI::IsHidden().

nekr.fabula@ did the crash reproduce for you on Chrome Canary/Beta too? The crash report is from Chrome 59.

From the first look it seems like the remote player sends an IPC back to WMPI via RendererMediaPlayerManager, but WMPI or at least the delegate is already destroyed so handling the message crashes.
Yes, I had it on every Android device I had with all Chromes: Chrome/Chrome Beta/Chrome Canary.

Here are crashes from my Chrome Beta: https://pastebin.com/Wfn3mEHk
I believe all them are from this issue.

If I'll be able to find correct (with the crashes) revision of the PWA I'll deploy that to a new URL.
Cc: tguilbert@chromium.org
Components: Internals>Media
I looked through the crash reports. Some of them contain RendererMediaPlayerManager in the stack, some of them don't. There're about 4 distinct stack traces there.

Most crashes are from Chrome 60.0.3112.50 (only pasted the top few methods from the stack trace):

http://crash/88ab9f4f38000000

libunwind::Registers_arm::getRegisterName(int)
blink::LocalDOMWindow::AcceptLanguagesChanged()
content::RendererMediaPlayerManager::OnMessageReceived(IPC::Message const&)
...

http://crash/a962ba7608000000

media::PipelineController::GetStatistics() const
media::WebMediaPlayerImpl::GetPipelineStatistics() const
media::WebMediaPlayerImpl::FinishMemoryUsageReport(long long)
media::WebMediaPlayerImpl::ReportMemoryUsage()
base::Timer::RunScheduledTask()
...

http://crash/91b3745e40000000

base::Timer::RunScheduledTask()
...

http://crash/c9b6f2cf38000000

media::WebMediaPlayerImpl::UpdatePlayState()
content::RendererMediaPlayerManager::OnMessageReceived(IPC::Message const&)
content::RenderFrameImpl::OnMessageReceived(IPC::Message const&)

The original crash report (http://crash/6e7d5ee938000000) is for Chrome 59.0.3071.125 with the stack of:

media::WebMediaPlayerImpl::IsHidden() const
media::WebMediaPlayerImpl::UpdatePlayState()
content::RendererMediaPlayerManager::OnMessageReceived(IPC::Message const&)
content::RenderFrameImpl::OnMessageReceived(IPC::Message const&)
...

These are the distinct stack traces from the pastebin crash report ids.
Cc: mlamouri@chromium.org fbeaufort@chromium.org
Status: Assigned (was: Unconfirmed)
Status: WontFix (was: Assigned)
I tried to create a repro page based on the OP description: https://avayvod.github.io/remote-playback/crash-new-document.html

It crashes _neither_ on Chrome Stable nor on debug ToT build when I cast the second video after navigating back.

The crash reports are all over the place and might mean a more generic problem with IPC / destruction race or could have their separate reasons.

I consider this bug a WontFix until a reliable repro is found.

nekr.fabula@gmail.com feel free to look at my repro page and describe what's missing compared to your test page so the crash reproduces.
Interesting how Browser bugs solving is often treated as something web developments should care about, like it's more important to them than to browser developers.

I stopped filing bugs into WebKit long time ago because it's always "Why we should do/change/fix that?" response.

WontFix it be, not my problem.
I'm sorry you feel this way. Fixing crashes is certainly a priority for browser developers and I for sure didn't mean to say web developers should work around the crashes by doing things you ended up doing in #c6.

However, fixing this particular bug without being able to reproduce it or having a good crash report is tricky. I could do some random code changes but how would I verify they helped without a repro case?

Again, if you have any suggestions on how to improve my test page from #c15, I'm happy to apply them and fix the bug if it reproduces. I understand if you don't have resources and/or further intent to help with that and really appreciate your replies and help so far.

Given the four different stack traces from the crash reports you provided, it could be you hit some other bug or bugs that manifested itself while casting videos from your test page. The crashes could have even been fixed since then.

"WontFix" means I don't intend to spend more time on this bug as of now, without any additional information, not that I think it has to be worked around forever. If the cause of the crashes is not fixed, it will manifest itself and will get fixed eventually, esp. if we'll get better stack traces and repro cases.
Here is a build from July 1: https://omroep-pwa-kndwmvxupb.now.sh
Following steps from the report, Chrome crashes every time.


Status: Started (was: WontFix)
Thanks. I confirm I can repro the crash now on Chrome from Stable to Canary but not on my local Chromium build. Will investigate if it got fixed between the last Canary/Dev and ToT or if it's only triggered on the official release builds.
Great, thank you!
avayvod@, we cut our stable candidate for M60 tonight at 5 PM PT, what's the status here?  Does this need to block?
amineer@ given that the crash happens in M59, I'm not sure it needs to block stable. I think it's fair to target M61 here.
I agree it is not a blocker. Even though it's very annoying to deal with it, there is a workaround.
Labels: -ReleaseBlock-Stable
Thanks!  And sorry for the hassle nekr.fabula@ (and for all the details).
Thanks to a debugging session over hangouts with Thomas it seems like we found the problem and a patch that prevents the crash, however it might do that by creating a memory leak so we'll look into it further tomorrow.

I've uploaded https://chromium-review.googlesource.com/c/576473/ to see if it breaks any tests.
I think the fix should do for now and can even be merged to a branch.
We don't seem to clean up remote media player bridge properly when disconnecting bug I feel this can be fixed in a separate cl.
Project Member

Comment 27 by bugdroid1@chromium.org, Jul 19 2017

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

commit fad2f3ea3dce44b8822679e28041ee34ebc44bde
Author: Anton Vayvod <avayvod@google.com>
Date: Wed Jul 19 21:45:27 2017

[Android, Cast] Fix crash when the second video is cast.

HTMLMediaElement may destroy WebMediaPlayerImpl when
handling disconnect from a remote device so WMPI shouldn't
do anything after calls to |client_|.

BUG= 738677 
TEST=follow the steps from the bug with the link from #c18

Change-Id: I59b299acc9740644bb046737419066734a870dc4
Reviewed-on: https://chromium-review.googlesource.com/576473
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Commit-Queue: Anton Vayvod <avayvod@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488001}
[modify] https://crrev.com/fad2f3ea3dce44b8822679e28041ee34ebc44bde/media/blink/webmediaplayer_impl.cc

Cc: candr...@chromium.org
Christine, could someone verify this please once the fix is in Canary?
Happy to merge afterwards (though I realize M60 might be too late).
Labels: Merge-Request-60
Tested in Chrome Canary 61.0.3162.0 and the crash is not reproducing.
Project Member

Comment 30 by sheriffbot@chromium.org, Jul 20 2017

Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: We are only 4 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-60 Merge-Rejected-60
Sorry, only super critical fixes for M60 at this time.
Labels: -M-60 M-61
Status: Fixed (was: Started)

Sign in to add a comment