media: Don't remove players from chrome://media-internals on process exit.

As long as chrome://media-internals is open, it should preserve log
entries. Log entries are still removed from the backing store, so these
players will be removed if the page is reloaded.

This CL also changes the trigger for backing store removal to include
tab crashes. Previously there was no reliable way to remove a logs for a
player from a crashed tab, which is problematic if the tab is incognito.

Bug: 741132
Change-Id: I92b152fa6578c3728ab22a45786e188d93fb87f2
Reviewed-on: https://chromium-review.googlesource.com/567644
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486159}

content/browser/media/media_internals.cc

@@ -785,6 +785,10 @@ MediaInternals::MediaInternals()
     : can_update_(false),
       owner_ids_(),
       uma_handler_(new MediaInternalsUMAHandler(this)) {
+  // TODO(sandersd): Is there ever a relevant case where TERMINATED is sent
+  // without CLOSED also being sent?
+  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_CLOSED,
+                 NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_TERMINATED,
                  NotificationService::AllBrowserContextsAndSources());
 }
@@ -795,10 +799,9 @@ void MediaInternals::Observe(int type,
                              const NotificationSource& source,
                              const NotificationDetails& details) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(type, NOTIFICATION_RENDERER_PROCESS_TERMINATED);
   RenderProcessHost* process = Source<RenderProcessHost>(source).ptr();
-
   uma_handler_->OnProcessTerminated(process->GetID());
+  // TODO(sandersd): Send a termination event before clearing the log.
   saved_events_by_process_.erase(process->GetID());
 }
 

content/browser/media/media_internals_proxy.cc

@@ -4,40 +4,18 @@
 
 #include "content/browser/media/media_internals_proxy.h"
 
-#include <stddef.h>
-
-#include <utility>
-
 #include "base/bind.h"
-#include "base/bind_helpers.h"
 #include "base/location.h"
-#include "base/macros.h"
-#include "base/single_thread_task_runner.h"
-#include "base/threading/thread_task_runner_handle.h"
+#include "content/browser/media/media_internals.h"
 #include "content/browser/media/media_internals_handler.h"
-#include "content/public/browser/content_browser_client.h"
-#include "content/public/browser/notification_service.h"
-#include "content/public/browser/notification_types.h"
-#include "content/public/browser/render_process_host.h"
-#include "content/public/browser/web_ui.h"
+#include "content/public/browser/browser_thread.h"
 
 namespace content {
 
 MediaInternalsProxy::MediaInternalsProxy() {
-  registrar_.Add(this, NOTIFICATION_RENDERER_PROCESS_TERMINATED,
-                 NotificationService::AllBrowserContextsAndSources());
 }
 
-void MediaInternalsProxy::Observe(int type,
-                                  const NotificationSource& source,
-                                  const NotificationDetails& details) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK_EQ(type, NOTIFICATION_RENDERER_PROCESS_TERMINATED);
-  RenderProcessHost* process = Source<RenderProcessHost>(source).ptr();
-  CallJavaScriptFunctionOnUIThread(
-      "media.onRendererTerminated",
-      base::MakeUnique<base::Value>(process->GetID()));
-}
+MediaInternalsProxy::~MediaInternalsProxy() {}
 
 void MediaInternalsProxy::Attach(MediaInternalsMessageHandler* handler) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ -65,8 +43,6 @@ void MediaInternalsProxy::GetEverything() {
       base::Bind(&MediaInternalsProxy::GetEverythingOnIOThread, this));
 }
 
-MediaInternalsProxy::~MediaInternalsProxy() {}
-
 void MediaInternalsProxy::GetEverythingOnIOThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   // TODO(xhwang): Investigate whether we can update on UI thread directly.
@@ -81,14 +57,4 @@ void MediaInternalsProxy::UpdateUIOnUIThread(const base::string16& update) {
     handler_->OnUpdate(update);
 }
 
-void MediaInternalsProxy::CallJavaScriptFunctionOnUIThread(
-    const std::string& function,
-    std::unique_ptr<base::Value> args) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  std::vector<const base::Value*> args_vector;
-  args_vector.push_back(args.get());
-  base::string16 update = WebUI::GetJavascriptCall(function, args_vector);
-  UpdateUIOnUIThread(update);
-}
-
 }  // namespace content

content/browser/media/media_internals_proxy.h

@@ -5,19 +5,12 @@
 #ifndef CONTENT_BROWSER_MEDIA_MEDIA_INTERNALS_PROXY_H_
 #define CONTENT_BROWSER_MEDIA_MEDIA_INTERNALS_PROXY_H_
 
-#include <memory>
-
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/sequenced_task_runner_helpers.h"
+#include "base/strings/string16.h"
 #include "content/browser/media/media_internals.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-
-namespace base {
-class Value;
-}  // namespace base
 
 namespace content {
 class MediaInternalsMessageHandler;
@@ -28,16 +21,10 @@ class MediaInternalsMessageHandler;
 // threads before destruction.
 class MediaInternalsProxy
     : public base::RefCountedThreadSafe<MediaInternalsProxy,
-                                        BrowserThread::DeleteOnUIThread>,
-      public NotificationObserver {
+                                        BrowserThread::DeleteOnUIThread> {
  public:
   MediaInternalsProxy();
 
-  // NotificationObserver implementation.
-  void Observe(int type,
-               const NotificationSource& source,
-               const NotificationDetails& details) override;
-
   // Register a Handler and start receiving callbacks from MediaInternals.
   void Attach(MediaInternalsMessageHandler* handler);
 
@@ -50,19 +37,14 @@ class MediaInternalsProxy
  private:
   friend struct BrowserThread::DeleteOnThread<BrowserThread::UI>;
   friend class base::DeleteHelper<MediaInternalsProxy>;
-  ~MediaInternalsProxy() override;
+  virtual ~MediaInternalsProxy();
 
   void GetEverythingOnIOThread();
 
   // Callback for MediaInternals to update. Must be called on UI thread.
   void UpdateUIOnUIThread(const base::string16& update);
 
-  // Call a JavaScript function on the page.
-  void CallJavaScriptFunctionOnUIThread(const std::string& function,
-                                        std::unique_ptr<base::Value> args);
-
   MediaInternalsMessageHandler* handler_;
-  NotificationRegistrar registrar_;
   MediaInternals::UpdateCallback update_callback_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaInternalsProxy);

content/browser/resources/media/main.js

@@ -27,14 +27,6 @@ var media = (function() {
     manager.updateVideoCaptureCapabilities(videoCaptureCapabilities)
   }
 
-  media.onRendererTerminated = function(renderId) {
-    util.object.forEach(manager.players_, function(playerInfo, id) {
-      if (playerInfo.properties['render_id'] == renderId) {
-        manager.removePlayer(id);
-      }
-    });
-  };
-
   media.updateAudioComponent = function(component) {
     var uniqueComponentId = component.owner_id + ':' + component.component_id;
     switch (component.status) {

content/test/data/media/webui/integration_test.html

@@ -50,14 +50,6 @@ found in the LICENSE file.
         assertEquals(event.params.fps, info.properties.fps);
       };
 
-      // Remove a player.
-      window.testOnRenderTerminated = function() {
-        window.testOnMediaEvent();
-
-        window.media.onRendererTerminated(TEST_RENDERER);
-        assertEquals(undefined, window.manager.players_[TEST_NAME]);
-      };
-
       window.testAudioComponents = function() {
         var event = {
           component_id: 1,