content: convert child process notifications to observer usage

content/browser/media/webrtc_internals.h

 // Copyright (c) 2013 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.
 
 #ifndef CONTENT_BROWSER_MEDIA_WEBRTC_INTERNALS_H_
 #define CONTENT_BROWSER_MEDIA_WEBRTC_INTERNALS_H_
 
 #include "base/memory/singleton.h"
 #include "base/observer_list.h"
 #include "base/process.h"
 #include "base/values.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/browser_child_process_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 
 namespace content {
 class WebRTCInternalsUIObserver;
 
 // This is a singleton class running in the browser UI thread.
 // It collects peer connection infomation from the renderers,
 // forwards the data to WebRTCInternalsUIObserver and
 // sends data collecting commands to the renderers.
-class CONTENT_EXPORT WebRTCInternals : public NotificationObserver{
+class CONTENT_EXPORT WebRTCInternals : public BrowserChildProcessObserver,
+                                       public NotificationObserver{
  public:
   static WebRTCInternals* GetInstance();
 
   // This method is called when a PeerConnection is created.
   // |render_process_id| is the id of the render process (not OS pid), which is
   // needed because we might not be able to get the OS process id when the
   // render process terminates and we want to clean up.
   // |pid| is the renderer process id, |lid| is the renderer local id used to
   // identify a PeerConnection, |url| is the url of the tab owning the
   // PeerConnection, |servers| is the servers configuration, |constraints| is
@@ skipping 22 matching lines @@
   // local id, |value| is the list of stats reports.
   void AddStats(base::ProcessId pid, int lid, const base::ListValue& value);
 
   // Methods for adding or removing WebRTCInternalsUIObserver.
   void AddObserver(WebRTCInternalsUIObserver *observer);
   void RemoveObserver(WebRTCInternalsUIObserver *observer);
 
   // Sends all update data to the observers.
   void SendAllUpdates();
 
+  // BrowserChildProcessObserver implementation.

[jam, 2013/02/19 18:07:09]

nit: if the methods to implement an interface can be made private, better to do
so so that the public parts of the class is smaller. also move after the
NotificationObserver method

[Paweł Hajdan Jr., 2013/02/20 12:11:42]

Even though I've seen it being done in some parts of the codebase, I don't
remember it being discussed on chromium-dev, and most places don't do that.

Additionally, an "evil" caller could cast to the parent type anyway. See the
following links:
http://stackoverflow.com/questions/484592/overriding-public-virtual-functions...
http://www.parashift.com/c++-faq-lite/hiding-inherited-public.html

If you still consider the above to be blocking this CL, please raise this on
chromium-dev.

[jam, 2013/02/20 19:26:31]

I realize the methods are still reachable through casting. But it makes it more
obvious to developers/reviewers that something weird is happening. These methods
are really implementation details of this class, and as such there's no reason
to expose them in the public section of the class. I consider this good hygiene
(exposing only what's needed).

[Paweł Hajdan Jr., 2013/02/27 19:13:23]

Alright, I've moved things to private when consistent with existing code. I've
tried to make the order of declarations also match the order of inheritance.

Please let me know if there are any remaining issues. For trivial ones, ideally
please give a positive review conditional on fixing these issues.

+  virtual void BrowserChildProcessCrashed(
+      const ChildProcessData& data) OVERRIDE;
+
  private:
   friend struct DefaultSingletonTraits<WebRTCInternals>;
 
   WebRTCInternals();
   virtual ~WebRTCInternals();
 
   void SendUpdate(const std::string& command, base::Value* value);
 
   // NotificationObserver implementation.
   virtual void Observe(int type,
                        const NotificationSource& source,
                        const NotificationDetails& details) OVERRIDE;
 
+  // Called when a renderer exits (including crashes).
+  void OnRendererExit(int render_process_id);
+
   ObserverList<WebRTCInternalsUIObserver> observers_;
 
   // |peer_connection_data_| is a list containing all the PeerConnection
   // updates.
   // Each item of the list represents the data for one PeerConnection, which
   // contains these fields:
   // "pid" -- processId of the renderer that creates the PeerConnection.
   // "lid" -- local Id assigned to the PeerConnection.
   // "url" -- url of the web page that created the PeerConnection.
   // "servers" and "constraints" -- server configuration and media constraints
   // used to initialize the PeerConnection respectively.
   // "log" -- a ListValue contains all the updates for the PeerConnection. Each
   // list item is a DictionaryValue containing "type" and "value", both of which
   // are strings.
   base::ListValue peer_connection_data_;
 
   NotificationRegistrar registrar_;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_MEDIA_WEBRTC_INTERNALS_H_