[MM] Fix random crashes and flickering while playing video.
authormsu.koo <msu.koo@samsung.com>
Mon, 21 Dec 2015 11:01:41 +0000 (20:01 +0900)
committerYoungsoo Choi <kenshin.choi@samsung.com>
Tue, 10 Jul 2018 07:55:23 +0000 (07:55 +0000)
commit3e30422bd4ba937920ec2551ef7d1e5c5d628b1b
tree6e5195c8490011d15e7bc75c8e3571b306afb502
parent5d0d55f31695b6227847ab04f6b3b03fbe6d68c8
[MM] Fix random crashes and flickering while playing video.

Random crash and flickering happens because
- Lifecycle between BrowserMediaPlayerManagerEfl(BMPMEfl) and
MediaPlayerEfls(MPEfl) is not properly managed.
- Lifecycle of delivered media_packet is not properly managed so that
Renderer occasionally accesses media_packet which was already destroyed.

Bundle of patches fix theses issues by
- Introducing weak pointer when posting tasks not to make task outlive
than tasker.
- Making MPEfl's lifecycle dependent with BMPMEfl.
- Introducing IPC for destroying media_packet explicitly after Renderer
uses it.
- Introducing scoped_ptr for media_packet not to make leaks even task
is cancelled.

In detail, this patch consists of following beta/m47 patches:
1. http://165.213.202.130/gerrit/#/c/100528/ by msu.koo
Previously, crash happens on refresh while playing videos using HTML's
iframe tag, because |OnNewTbmBufferAvailable| is queued after RenderFrame
is deleted, so it refers already-deleted RenderFrame.
This patch includes 2 major changes.
- On Renderer-side,
This separates OnNewTbmBufferAvailable() from RenderFrame's lifecycle.
- On Browser-side,
This introduces BrowserMediaPacketManager which is Control IPC handler
outlives than BrowsermediaPlayerManager. This class can handle releasing
|media_packet| without affected by Player's lifecycle.

2. http://165.213.202.130/gerrit/#/c/100838/ by sangdeug.kim
This patch fixes crash during run |MediaPlayerEfl::DeliverMediaPacket| by
checking the destructing state.

3. http://165.213.202.130/gerrit/#/c/100839/ by msu.koo
Previously, random crash happens because player accesses manager
without considering the manager's lifecycle.
Currently, BrowserMediaPlayerManager request to delete Players
when it's deconstructed, which means that Player can outlive
than Manager.
This fix introduces WeakPtr and let Player access Manager
via WeakPtr for lifetime handling between Player and Manager.

4. http://165.213.202.130/gerrit/#/c/102313/ by msu.koo
"Improve MediaPlayerEfl lifecycle management."
Previously, MediaPlayerEfl outlives than BrowserMediaPlayerManagerEfl
so additional checking if Manager is alive before callback.
This patch make MediaPlayerEfl's lifetime dependant with BMPMEfl
and remove above checking routines.
Also remove IsPlayerDestructing and relates, which is no more required
after introducing WeakPtr for PostTask.

5. http://165.213.202.130/gerrit/#/c/102463/ by msu.koo
"Fix possible |media_packet| leaks when DeliverMediaPacket is cancelled."
Previously, DeliverMediaPacket task can be cancelled if queued after
MediaPlayerEfl is destroyed. But when DeliverMediaPacket is cancelled,
there is no handling to destroy the passed |media_packet| on it, so it
causes |media_packet| leaks.
This patch introduces MediaPacketProxy and scoped_ptr to fix it.

6. http://165.213.202.130/gerrit/#/c/103502/ by sangdeug.kim
After making MediaPlayerEfl's lifetime dependent with
BrowserMediaPlayerManager(BMPM), crash happens while destructing player.
Root cause is
- WeakPtr of BMPM is invalidated before Players are destroyed
by the destruction sequence of member variables.
- While destructing Players, Player accesses BMPM
via already-invalidated WeakPtr -> Crash happens.
This patch removes redundant WeakPtr of BMPM and related codes,
and let Player access Manager with raw pointer.
This patch also ensure destroy all players before BMPM is destroyed.

7. http://165.213.202.130/gerrit/#/c/103039/ by msu.koo
Previous implementation implements Proxy Class for media_packet,
but it can be replaced with Chromium's scoped_ptr.

Together with: Iecc266fee83ae009285554dce4581d6c7e9bf545

Bug: http://10.113.136.204/jira/browse/TSAM-751
Bug: http://10.113.136.204/jira/browse/TSAM-836
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15296
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15410
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15425
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15439
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15445
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15458
Bug: http://web.sec.samsung.net/bugzilla/show_bug.cgi?id=15502

Reviewed by: msu.koo, sm.venugopal, sns.park

Change-Id: I59747d19e18cdcb8708d6ba91c49bc2eb7fad66b
Signed-off-by: msu.koo <msu.koo@samsung.com>
Signed-off-by: sangdeug.kim <sangdeug.kim@samsung.com>
17 files changed:
tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.cc [new file with mode: 0644]
tizen_src/chromium_impl/content/browser/media/browser_mediapacket_manager.h [new file with mode: 0644]
tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.cc
tizen_src/chromium_impl/content/browser/media/efl/browser_media_player_manager_efl.h
tizen_src/chromium_impl/content/browser/media/media_web_contents_observer_efl.cc
tizen_src/chromium_impl/content/common/media/efl/media_player_messages_efl.h
tizen_src/chromium_impl/content/content_browser_efl.gypi
tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.cc
tizen_src/chromium_impl/content/renderer/media/efl/renderer_media_player_manager_efl.h
tizen_src/chromium_impl/media/base/efl/media_player_efl.cc
tizen_src/chromium_impl/media/base/efl/media_player_efl.h
tizen_src/chromium_impl/media/base/efl/media_player_util_efl.cc
tizen_src/chromium_impl/media/base/efl/media_player_util_efl.h
tizen_src/chromium_impl/media/base/tizen/media_player_bridge_capi.cc
tizen_src/chromium_impl/media/base/tizen/media_player_bridge_capi.h
tizen_src/chromium_impl/media/base/tizen/media_source_player_capi.cc
tizen_src/chromium_impl/media/base/tizen/media_source_player_capi.h