Hide Media Remoting UI when media player is cleared.

When WebMediaPlayer is destroyed while remoting, the remoting UI should be hidden. Previous approach (https://chromium-review.googlesource.com/c/581950) added the notification in RendererController's destructor, which requires proper destruct order in WebMediaPlayerImpl, and potentially cause crash. This CL reverted previous change and instead hide the remoting UI directly in media element when media player is cleared. [email protected] (cherry picked from commit 8729914) Bug: 761699 Change-Id: I10452f93785a8e5814b4cffe466846d55962fa1e Reviewed-on: https://chromium-review.googlesource.com/600950 Commit-Queue: Yuri Wiitala <[email protected]> Reviewed-by: Mounir Lamouri <[email protected]> Reviewed-by: Yuri Wiitala <[email protected]> Reviewed-by: Chrome Cunningham <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#501741} Reviewed-on: https://chromium-review.googlesource.com/676756 Reviewed-by: Xiangjun Zhang <[email protected]> Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#381} Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
tenta-browser · Sep 21, 2017 · 4d8d620 · 4d8d620
1 parent cace7aa
commit 4d8d620
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 26 deletions.
diff --git a/media/blink/webmediaplayer_impl.cc b/media/blink/webmediaplayer_impl.cc
@@ -343,6 +343,8 @@ WebMediaPlayerImpl::~WebMediaPlayerImpl() {
   // Destruct compositor resources in the proper order.
   client_->SetWebLayer(nullptr);
 
+  client_->MediaRemotingStopped();
+
   if (!surface_layer_for_video_enabled_ && video_weblayer_) {
     static_cast<cc::VideoLayer*>(video_weblayer_->layer())->StopUsingProvider();
   }

diff --git a/media/remoting/renderer_controller.cc b/media/remoting/renderer_controller.cc
@@ -44,10 +44,6 @@ RendererController::RendererController(scoped_refptr<SharedSession> session)
 
 RendererController::~RendererController() {
   DCHECK(thread_checker_.CalledOnValidThread());
-  if (remote_rendering_started_) {
-    DCHECK(client_);
-    client_->SwitchToLocalRenderer();
-  }
   metrics_recorder_.WillStopSession(MEDIA_ELEMENT_DESTROYED);
   session_->RemoveClient(this);
 }

diff --git a/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp b/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
@@ -67,7 +67,6 @@ enum VideoPersistenceControlsType {
 
 inline HTMLVideoElement::HTMLVideoElement(Document& document)
     : HTMLMediaElement(videoTag, document),
-      media_remoting_status_(MediaRemotingStatus::kNotStarted),
       remoting_interstitial_(nullptr) {
   if (document.GetSettings()) {
     default_poster_url_ =
@@ -504,8 +503,6 @@ ScriptPromise HTMLVideoElement::CreateImageBitmap(
 
 void HTMLVideoElement::MediaRemotingStarted(
     const WebString& remote_device_friendly_name) {
-  DCHECK(media_remoting_status_ == MediaRemotingStatus::kNotStarted);
-  media_remoting_status_ = MediaRemotingStatus::kStarted;
   if (!remoting_interstitial_) {
     remoting_interstitial_ = new MediaRemotingInterstitial(*this);
     ShadowRoot& shadow_root = EnsureUserAgentShadowRoot();
@@ -516,12 +513,8 @@ void HTMLVideoElement::MediaRemotingStarted(
 }
 
 void HTMLVideoElement::MediaRemotingStopped() {
-  DCHECK(media_remoting_status_ == MediaRemotingStatus::kDisabled ||
-         media_remoting_status_ == MediaRemotingStatus::kStarted);
-  if (media_remoting_status_ != MediaRemotingStatus::kDisabled)
-    media_remoting_status_ = MediaRemotingStatus::kNotStarted;
-  DCHECK(remoting_interstitial_);
-  remoting_interstitial_->Hide();
+  if (remoting_interstitial_)
+    remoting_interstitial_->Hide();
 }
 
 WebMediaPlayer::DisplayType HTMLVideoElement::DisplayType() const {
@@ -531,9 +524,12 @@ WebMediaPlayer::DisplayType HTMLVideoElement::DisplayType() const {
 }
 
 void HTMLVideoElement::DisableMediaRemoting() {
-  media_remoting_status_ = MediaRemotingStatus::kDisabled;
   if (GetWebMediaPlayer())
     GetWebMediaPlayer()->RequestRemotePlaybackDisabled(true);
 }
 
+bool HTMLVideoElement::IsRemotingInterstitialVisible() const {
+  return remoting_interstitial_ && remoting_interstitial_->IsVisible();
+}
+
 }  // namespace blink
diff --git a/third_party/WebKit/Source/core/html/HTMLVideoElement.h b/third_party/WebKit/Source/core/html/HTMLVideoElement.h
@@ -55,8 +55,6 @@ class CORE_EXPORT HTMLVideoElement final : public HTMLMediaElement,
 
   bool HasPendingActivity() const final;
 
-  enum class MediaRemotingStatus { kNotStarted, kStarted, kDisabled };
-
   // Node override.
   Node::InsertionNotificationRequest InsertedInto(ContainerNode*) override;
   void RemovedFrom(ContainerNode*) override;
@@ -140,9 +138,7 @@ class CORE_EXPORT HTMLVideoElement final : public HTMLMediaElement,
 
   bool IsPersistent() const;
 
-  MediaRemotingStatus GetMediaRemotingStatus() const {
-    return media_remoting_status_;
-  }
+  bool IsRemotingInterstitialVisible() const;
   void DisableMediaRemoting();
 
   void MediaRemotingStarted(const WebString& remote_device_friendly_name) final;
@@ -178,8 +174,6 @@ class CORE_EXPORT HTMLVideoElement final : public HTMLMediaElement,
   Member<MediaCustomControlsFullscreenDetector>
       custom_controls_fullscreen_detector_;
 
-  MediaRemotingStatus media_remoting_status_;
-
   Member<MediaRemotingInterstitial> remoting_interstitial_;
 
   AtomicString default_poster_url_;

diff --git a/third_party/WebKit/Source/core/html/media/MediaRemotingInterstitial.cpp b/third_party/WebKit/Source/core/html/media/MediaRemotingInterstitial.cpp
@@ -48,7 +48,8 @@ MediaRemotingInterstitial::MediaRemotingInterstitial(
 
 void MediaRemotingInterstitial::Show(
     const WebString& remote_device_friendly_name) {
-  DCHECK(!should_be_visible_);
+  if (should_be_visible_)
+    return;
   if (remote_device_friendly_name.IsEmpty()) {
     cast_text_message_->setInnerText(
         GetVideoElement().GetLocale().QueryString(
@@ -71,7 +72,8 @@ void MediaRemotingInterstitial::Show(
 }
 
 void MediaRemotingInterstitial::Hide() {
-  DCHECK(should_be_visible_);
+  if (!should_be_visible_)
+    return;
   if (toggle_insterstitial_timer_.IsActive())
     toggle_insterstitial_timer_.Stop();
   should_be_visible_ = false;

diff --git a/third_party/WebKit/Source/core/html/media/MediaRemotingInterstitial.h b/third_party/WebKit/Source/core/html/media/MediaRemotingInterstitial.h
@@ -40,6 +40,9 @@ class MediaRemotingInterstitial final : public HTMLDivElement {
 
   void OnPosterImageChanged();
 
+  // Query for whether the remoting interstitial is visible.
+  bool IsVisible() const { return should_be_visible_; }
+
   HTMLVideoElement& GetVideoElement() const { return *video_element_; }
 
   DECLARE_VIRTUAL_TRACE();
@@ -52,7 +55,7 @@ class MediaRemotingInterstitial final : public HTMLDivElement {
   void ToggleInterstitialTimerFired(TimerBase*);
 
   // Indicates whether the interstitial should be visible. It is set/changed
-  // when SHow()/Hide() is called.
+  // when Show()/Hide() is called.
   bool should_be_visible_ = false;
 
   TaskRunnerTimer<MediaRemotingInterstitial> toggle_insterstitial_timer_;

diff --git a/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp b/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
@@ -607,8 +607,7 @@ void MediaControlsImpl::MakeTransparent() {
 bool MediaControlsImpl::ShouldHideMediaControls(unsigned behavior_flags) const {
   // Never hide for a media element without visual representation.
   if (!MediaElement().IsHTMLVideoElement() || !MediaElement().HasVideo() ||
-      toHTMLVideoElement(MediaElement()).GetMediaRemotingStatus() ==
-          HTMLVideoElement::MediaRemotingStatus::kStarted) {
+      toHTMLVideoElement(MediaElement()).IsRemotingInterstitialVisible()) {
     return false;
   }