Send a notice that the video player is destroyed to MediaLog

This happens synchronously with the destruction of WebMediaPlayerImpl
so that the devtools backend for MediaLog knows it shouldn't be sending
events since the page may have been destroyed. Events that happen
after WEBMEDIAPLAYER_DESTROYED get dropped from _only_ the devtools
version.

This should fix the nullptr deref bug caused by the destruction order
of the blink objects and the WebMediaPlayerImpl::DestructionHelper.

Bug: 794255
Bug: 1014433
Change-Id: I25e2110a31561367baf7104c77a8e5301b5afb0c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1891078
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711858}

content/renderer/media/batching_media_log.cc

@@ -65,6 +65,10 @@ BatchingMediaLog::~BatchingMediaLog() {
     SendQueuedMediaEvents();
 }
 
+void BatchingMediaLog::OnWebMediaPlayerDestroyedLocked() {
+  event_handler_->OnWebMediaPlayerDestroyed();
+}
+
 void BatchingMediaLog::AddEventLocked(
     std::unique_ptr<media::MediaLogEvent> event) {
   Log(event.get());

content/renderer/media/batching_media_log.h

@@ -35,6 +35,7 @@ class CONTENT_EXPORT BatchingMediaLog : public media::MediaLog {
    public:
     virtual ~EventHandler() = default;
     virtual void SendQueuedMediaEvents(std::vector<media::MediaLogEvent>) = 0;
+    virtual void OnWebMediaPlayerDestroyed() = 0;
   };
 
   BatchingMediaLog(const GURL& security_origin,
@@ -48,6 +49,7 @@ class CONTENT_EXPORT BatchingMediaLog : public media::MediaLog {
  protected:
   // MediaLog implementation.
   void AddEventLocked(std::unique_ptr<media::MediaLogEvent> event) override;
+  void OnWebMediaPlayerDestroyedLocked() override;
   std::string GetErrorMessageLocked() override;
 
  private:

content/renderer/media/batching_media_log_unittest.cc

@@ -22,6 +22,7 @@ class TestEventHandler : public BatchingMediaLog::EventHandler {
   explicit TestEventHandler(BatchingMediaLogTest* test_cls)
       : test_cls_(test_cls) {}
   void SendQueuedMediaEvents(std::vector<media::MediaLogEvent> events) override;
+  void OnWebMediaPlayerDestroyed() override;
 
  private:
   BatchingMediaLogTest* test_cls_;
@@ -79,6 +80,8 @@ void TestEventHandler::SendQueuedMediaEvents(
   test_cls_->AddEventsForTesting(events);
 }
 
+void TestEventHandler::OnWebMediaPlayerDestroyed() {}
+
 TEST_F(BatchingMediaLogTest, ThrottleSendingEvents) {
   AddEvent(media::MediaLogEvent::LOAD);
   EXPECT_EQ(0, message_count());

content/renderer/media/inspector_media_event_handler.cc

@@ -34,6 +34,10 @@ InspectorMediaEventHandler::InspectorMediaEventHandler(
 // this method is no longer needed. Refactor MediaLogEvent at some point.
 void InspectorMediaEventHandler::SendQueuedMediaEvents(
     std::vector<media::MediaLogEvent> events_to_send) {
+  // If the video player is gone, the whole frame
+  if (video_player_destroyed_)
+    return;
+
   blink::InspectorPlayerEvents events;
   blink::InspectorPlayerProperties properties;
 
@@ -77,4 +81,8 @@ void InspectorMediaEventHandler::SendQueuedMediaEvents(
     inspector_context_->SetPlayerProperties(player_id_, properties);
 }
 
+void InspectorMediaEventHandler::OnWebMediaPlayerDestroyed() {
+  video_player_destroyed_ = true;
+}
+
 }  // namespace content

content/renderer/media/inspector_media_event_handler.h

@@ -21,10 +21,12 @@ class CONTENT_EXPORT InspectorMediaEventHandler
   explicit InspectorMediaEventHandler(blink::MediaInspectorContext*);
   ~InspectorMediaEventHandler() override = default;
   void SendQueuedMediaEvents(std::vector<media::MediaLogEvent>) override;
+  void OnWebMediaPlayerDestroyed() override;
 
  private:
   blink::MediaInspectorContext* inspector_context_;
   blink::WebString player_id_;
+  bool video_player_destroyed_ = false;
 };
 
 }  // namespace content

content/renderer/media/render_media_event_handler.cc

@@ -13,4 +13,8 @@ void RenderMediaEventHandler::SendQueuedMediaEvents(
   RenderThread::Get()->Send(new ViewHostMsg_MediaLogEvents(events_to_send));
 }
 
+// This media log doesn't care, since the RenderThread outlives us for
+// chrome://media-internals.
+void RenderMediaEventHandler::OnWebMediaPlayerDestroyed() {}
+
 }  // namespace content

content/renderer/media/render_media_event_handler.h

@@ -19,6 +19,7 @@ class CONTENT_EXPORT RenderMediaEventHandler
   RenderMediaEventHandler() = default;
   ~RenderMediaEventHandler() override = default;
   void SendQueuedMediaEvents(std::vector<media::MediaLogEvent>) override;
+  void OnWebMediaPlayerDestroyed() override;
 };
 
 }  // namespace content

media/base/media_log.cc

@@ -173,6 +173,16 @@ MediaLog::~MediaLog() {
     InvalidateLog();
 }
 
+void MediaLog::OnWebMediaPlayerDestroyed() {
+  AddEvent(CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_DESTROYED));
+  base::AutoLock auto_lock(parent_log_record_->lock);
+  // Forward to the parent log's implementation.
+  if (parent_log_record_->media_log)
+    parent_log_record_->media_log->OnWebMediaPlayerDestroyedLocked();
+}
+
+void MediaLog::OnWebMediaPlayerDestroyedLocked() {}
+
 void MediaLog::AddEvent(std::unique_ptr<MediaLogEvent> event) {
   base::AutoLock auto_lock(parent_log_record_->lock);
   // Forward to the parent log's implementation.

media/base/media_log.h

@@ -79,6 +79,10 @@ class MEDIA_EXPORT MediaLog {
   // do something.
   void AddEvent(std::unique_ptr<MediaLogEvent> event);
 
+  // Notify the media log that the player is destroyed. Some implementations
+  // will want to change event handling based on this.
+  void OnWebMediaPlayerDestroyed();
+
   // Returns a string usable as the contents of a MediaError.message.
   // This method returns an incomplete message if it is called before the
   // pertinent events for the error have been added to the log.
@@ -143,6 +147,7 @@ class MEDIA_EXPORT MediaLog {
   //
   // Please see the documentation for the corresponding public methods.
   virtual void AddEventLocked(std::unique_ptr<MediaLogEvent> event);
+  virtual void OnWebMediaPlayerDestroyedLocked();
   virtual std::string GetErrorMessageLocked();
 
   // Notify all child logs that they should stop working.  This should be called

media/blink/webmediaplayer_impl.cc

@@ -457,8 +457,7 @@ WebMediaPlayerImpl::~WebMediaPlayerImpl() {
     video_layer_->StopUsingProvider();
 
   simple_watch_timer_.Stop();
-  media_log_->AddEvent(
-      media_log_->CreateEvent(MediaLogEvent::WEBMEDIAPLAYER_DESTROYED));
+  media_log_->OnWebMediaPlayerDestroyed();
 
   if (data_source_)
     data_source_->Stop();