media: Add BrowserCdmManagerProcessWatcher.

content/browser/media/cdm/browser_cdm_manager.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/media/cdm/browser_cdm_manager.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/lazy_instance.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/task_runner.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/permission_manager.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
-#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/web_contents.h"
 #include "media/base/browser_cdm.h"
 #include "media/base/browser_cdm_factory.h"
 #include "media/base/cdm_promise.h"
 #include "media/base/limits.h"
 
 #if defined(OS_ANDROID)
 #include "content/public/common/renderer_preferences.h"
 #endif
 
@@ skipping 67 matching lines @@
 template <>
 void CdmPromiseInternal<std::string>::resolve(const std::string& session_id) {
   MarkPromiseSettled();
   manager_->ResolvePromiseWithSession(render_frame_id_, cdm_id_, promise_id_,
                                       session_id);
 }
 
 typedef CdmPromiseInternal<> SimplePromise;
 typedef CdmPromiseInternal<std::string> NewSessionPromise;
 
-}  // namespace
-
 // Render process ID to BrowserCdmManager map.
 typedef std::map<int, BrowserCdmManager*> BrowserCdmManagerMap;
 base::LazyInstance<BrowserCdmManagerMap>::Leaky g_browser_cdm_manager_map =
     LAZY_INSTANCE_INITIALIZER;
 
+// Keeps the BrowserCdmManager alive, and in the global map, for as long as the
+// RenderProcessHost is connected to the child process. This class is a
+// self-owned observer.
+class BrowserCdmManagerProcessWatcher : public RenderProcessHostObserver {
+ public:
+  BrowserCdmManagerProcessWatcher(
+      int render_process_id,
+      const scoped_refptr<BrowserCdmManager>& manager)
+      : browser_cdm_manager_(manager) {
+    RenderProcessHost::FromID(render_process_id)->AddObserver(this);
+    CHECK(g_browser_cdm_manager_map.Get()
+              .insert(std::make_pair(render_process_id, manager.get()))
+              .second);

[ddorwin, 2015/07/09 17:52:22]

Is this checking that manager.get() is not NULL? Is there a reason not to do
that independent of the insert?

[xhwang, 2015/07/09 19:04:40]

This is to check that the insert succeeded, meaning that there wasn't an
existing entry in the map with |render_process_id|.

+  }
+
+  // RenderProcessHostObserver:
+  void RenderProcessExited(RenderProcessHost* host,
+                           base::TerminationStatus /* status */,
+                           int /* exit_code */) override {
+    Destroy(host);

[ddorwin, 2015/07/09 17:52:22]

nit: This looks like it destroys |host|.

It's a bit strange that we need a parameter to destroy |this|. Should we have
saved it at line 122 then DCHECKed it here and had no parameter for Destroy()?

Alternatively, s/Destroy/RemoveHostOberverAndDestroy/ or something like that.

[xhwang, 2015/07/09 19:04:39]

Hmm, all HostObservers know which Host they are observing. But still we have
|host| in RenderProcessExited() and RenderProcessHostDestroyed(). I think this
is root of this strangeness.
Done.

[ncarter (slow), 2015/07/09 19:16:22]

It's structured this way to allow a RPHObserver instance to be an observer of
multiple RPH's at the same time.

+  }
+
+  void RenderProcessHostDestroyed(RenderProcessHost* host) override {
+    Destroy(host);
+  }
+
+ private:
+  void Destroy(RenderProcessHost* host) {
+    CHECK(g_browser_cdm_manager_map.Get().erase(host->GetID()));
+    host->RemoveObserver(this);
+    delete this;
+  }
+
+  const scoped_refptr<BrowserCdmManager> browser_cdm_manager_;
+};
+
+}  // namespace
+
+// static
 BrowserCdmManager* BrowserCdmManager::FromProcess(int render_process_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  if (!g_browser_cdm_manager_map.Get().count(render_process_id))
-    return NULL;
-
-  return g_browser_cdm_manager_map.Get()[render_process_id];
+  auto& map = g_browser_cdm_manager_map.Get();
+  auto iterator = map.find(render_process_id);
+  return (iterator == map.end()) ? nullptr : iterator->second;
 }
 
 BrowserCdmManager::BrowserCdmManager(
     int render_process_id,
     const scoped_refptr<base::TaskRunner>& task_runner)
     : BrowserMessageFilter(CdmMsgStart),
       render_process_id_(render_process_id),
       task_runner_(task_runner),
       weak_ptr_factory_(this) {
   DVLOG(1) << __FUNCTION__ << ": " << render_process_id_;
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 

[ddorwin, 2015/07/09 17:52:22]

Can/should we check that there is no entry for the render_process_id? Can there
be an existing entry? If so, you might comment that. I don't think so, though,
because that would break FromProcess().

[xhwang, 2015/07/09 19:04:39]

This is indirectly checked on line 123.
No. Basically you should never have two BrowserCdmManagerProcessWatcher alive
for the same process_id.
Ack.

+  new BrowserCdmManagerProcessWatcher(render_process_id, this);
+
   if (!task_runner_.get()) {
     task_runner_ =
         BrowserThread::GetMessageLoopProxyForThread(BrowserThread::UI);
   }
-
-  DCHECK(!g_browser_cdm_manager_map.Get().count(render_process_id_))
-      << render_process_id_;
-  g_browser_cdm_manager_map.Get()[render_process_id] = this;
 }
 
 BrowserCdmManager::~BrowserCdmManager() {
   DVLOG(1) << __FUNCTION__ << ": " << render_process_id_;
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  DCHECK(g_browser_cdm_manager_map.Get().count(render_process_id_));
-  DCHECK_EQ(this, g_browser_cdm_manager_map.Get()[render_process_id_]);
-
-  g_browser_cdm_manager_map.Get().erase(render_process_id_);
+  DCHECK(g_browser_cdm_manager_map.Get().count(render_process_id_) == 0);
 }
 
 // Makes sure BrowserCdmManager is always deleted on the Browser UI thread.
 void BrowserCdmManager::OnDestruct() const {
   DVLOG(1) << __FUNCTION__ << ": " << render_process_id_;
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
     delete this;
   } else {
     BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, this);
   }
@@ skipping 484 matching lines @@
   BrowserCdm* cdm = GetCdm(render_frame_id, cdm_id);
   if (!cdm) {
     promise->reject(MediaKeys::INVALID_STATE_ERROR, 0, "CDM not found.");
     return;
   }
 
   cdm->LoadSession(session_type, session_id, promise.Pass());
 }
 
 }  // namespace content