Deliver webrtc-internals updates in bulk, every 500ms at most.

content/browser/media/webrtc_internals.cc

Index: content/browser/media/webrtc_internals.cc
diff --git a/content/browser/media/webrtc_internals.cc b/content/browser/media/webrtc_internals.cc
index 40aa91baaec23615a1cccddf46369c8e1dc25a35..9269012500bedaa2e9a131dd95a2467c0f790a33 100644
--- a/content/browser/media/webrtc_internals.cc
+++ b/content/browser/media/webrtc_internals.cc
@@ -39,10 +39,22 @@ static base::ListValue* EnsureLogList(base::DictionaryValue* dict) {
 
 }  // namespace
 
+WebRTCInternals::PendingUpdate::PendingUpdate(
+    const std::string& command, scoped_ptr<base::Value> value)
+    : command_(command), value_(std::move(value)) {}
+
+WebRTCInternals::PendingUpdate::PendingUpdate(PendingUpdate&& other)
+    : command_(std::move(other.command_)),
+      value_(std::move(other.value_)) {}
+
+WebRTCInternals::PendingUpdate::~PendingUpdate() {}
+
 WebRTCInternals::WebRTCInternals()
     : audio_debug_recordings_(false),
       event_log_recordings_(false),
-      selecting_event_log_(false) {
+      selecting_event_log_(false),
+      pending_updates_queued_(false),
+      weak_factory_(this) {
 // TODO(grunell): Shouldn't all the webrtc_internals* files be excluded from the
 // build if WebRTC is disabled?
 #if defined(ENABLE_WEBRTC)
@@ -82,9 +94,6 @@ void WebRTCInternals::OnAddPeerConnection(int render_process_id,
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
   base::DictionaryValue* dict = new base::DictionaryValue();
-  if (!dict)
-    return;
-
   dict->SetInteger("rid", render_process_id);
   dict->SetInteger("pid", static_cast<int>(pid));
   dict->SetInteger("lid", lid);
@@ -95,7 +104,7 @@ void WebRTCInternals::OnAddPeerConnection(int render_process_id,
   CreateOrReleasePowerSaveBlocker();
 
   if (observers_.might_have_observers())
-    SendUpdate("addPeerConnection", dict);
+    SendUpdate("addPeerConnection", dict->CreateDeepCopy());

[Henrik Grunell, 2016/02/08 09:52:08]

Why do you need to create a deep copy? Could the ownership own |dict| just be
transferred?

[tommi (sloooow) - chröme, 2016/02/08 13:50:23]

ownership of dict lies with peer_connection_data_ (see line 103), so we need a
copy.

[Henrik Grunell, 2016/02/09 08:18:11]

Acknowledged.

 
   if (render_process_id_set_.insert(render_process_id).second) {
     RenderProcessHost* host = RenderProcessHost::FromID(render_process_id);
@@ -122,10 +131,10 @@ void WebRTCInternals::OnRemovePeerConnection(ProcessId pid, int lid) {
     CreateOrReleasePowerSaveBlocker();
 
     if (observers_.might_have_observers()) {
-      base::DictionaryValue id;
-      id.SetInteger("pid", static_cast<int>(pid));
-      id.SetInteger("lid", lid);
-      SendUpdate("removePeerConnection", &id);
+      scoped_ptr<base::DictionaryValue> id(new base::DictionaryValue());
+      id->SetInteger("pid", static_cast<int>(pid));
+      id->SetInteger("lid", lid);
+      SendUpdate("removePeerConnection", std::move(id));
     }
     break;
   }
@@ -163,12 +172,12 @@ void WebRTCInternals::OnUpdatePeerConnection(
     log->Append(log_entry);
 
     if (observers_.might_have_observers()) {
-      base::DictionaryValue update;
-      update.SetInteger("pid", static_cast<int>(pid));
-      update.SetInteger("lid", lid);
-      update.MergeDictionary(log_entry);
+      scoped_ptr<base::DictionaryValue> update(new base::DictionaryValue());
+      update->SetInteger("pid", static_cast<int>(pid));
+      update->SetInteger("lid", lid);
+      update->MergeDictionary(log_entry);
 
-      SendUpdate("updatePeerConnection", &update);
+      SendUpdate("updatePeerConnection", std::move(update));
     }
     return;
   }
@@ -179,17 +188,13 @@ void WebRTCInternals::OnAddStats(base::ProcessId pid, int lid,
   if (!observers_.might_have_observers())
     return;
 
-  base::DictionaryValue dict;
-  dict.SetInteger("pid", static_cast<int>(pid));
-  dict.SetInteger("lid", lid);
-
-  base::ListValue* list = value.DeepCopy();
-  if (!list)
-    return;
+  scoped_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
+  dict->SetInteger("pid", static_cast<int>(pid));
+  dict->SetInteger("lid", lid);
 
-  dict.Set("reports", list);
+  dict->Set("reports", value.CreateDeepCopy());
 
-  SendUpdate("addStats", &dict);
+  SendUpdate("addStats", std::move(dict));
 }
 
 void WebRTCInternals::OnGetUserMedia(int rid,
@@ -213,7 +218,7 @@ void WebRTCInternals::OnGetUserMedia(int rid,
   get_user_media_requests_.Append(dict);
 
   if (observers_.might_have_observers())
-    SendUpdate("addGetUserMedia", dict);
+    SendUpdate("addGetUserMedia", dict->CreateDeepCopy());
 
   if (render_process_id_set_.insert(rid).second) {
     RenderProcessHost* host = RenderProcessHost::FromID(rid);
@@ -222,12 +227,12 @@ void WebRTCInternals::OnGetUserMedia(int rid,
   }
 }
 
-void WebRTCInternals::AddObserver(WebRTCInternalsUIObserver *observer) {
+void WebRTCInternals::AddObserver(WebRTCInternalsUIObserver* observer) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   observers_.AddObserver(observer);
 }
 
-void WebRTCInternals::RemoveObserver(WebRTCInternalsUIObserver *observer) {
+void WebRTCInternals::RemoveObserver(WebRTCInternalsUIObserver* observer) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   observers_.RemoveObserver(observer);
 
@@ -251,8 +256,8 @@ void WebRTCInternals::UpdateObserver(WebRTCInternalsUIObserver* observer) {
 
 void WebRTCInternals::EnableAudioDebugRecordings(
     content::WebContents* web_contents) {
-#if defined(ENABLE_WEBRTC)
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+#if defined(ENABLE_WEBRTC)
 #if defined(OS_ANDROID)
   EnableAudioDebugRecordingsOnAllRenderProcessHosts();
 #else
@@ -273,8 +278,8 @@ void WebRTCInternals::EnableAudioDebugRecordings(
 }
 
 void WebRTCInternals::DisableAudioDebugRecordings() {
-#if defined(ENABLE_WEBRTC)
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+#if defined(ENABLE_WEBRTC)
   audio_debug_recordings_ = false;
 
   // Tear down the dialog since the user has unchecked the audio debug
@@ -352,12 +357,20 @@ void WebRTCInternals::ResetForTesting() {
   audio_debug_recordings_ = false;
 }
 
-void WebRTCInternals::SendUpdate(const string& command, base::Value* value) {
+void WebRTCInternals::SendUpdate(const string& command,
+                                 scoped_ptr<base::Value> value) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(observers_.might_have_observers());
 
-  FOR_EACH_OBSERVER(WebRTCInternalsUIObserver,
-                    observers_,
-                    OnUpdate(command, value));
+  pending_updates_.push(PendingUpdate(command, std::move(value)));
+
+  if (!pending_updates_queued_) {

[Henrik Grunell, 2016/02/08 09:52:08]

Could you just check is queue is empty? (Before pushing of course.)

[tommi (sloooow) - chröme, 2016/02/08 13:50:23]

We won't be empty on entry in many cases. E.g. in the example case from the bug,
we might have tens of queued up updates already when the method is called.

[Henrik Grunell, 2016/02/09 08:18:11]

I don't really understand how this works I guess. Isn't this the only place were
updates are pushed to the queue? If that's the case, we'll always post the task
when the first update is pushed, no?

[tommi (sloooow) - chröme, 2016/02/09 09:34:16]

Ah yes of course you're right, sorry I didn't get it the first time.  Since
we're always on the UI thread we don't need the extra bool. Done.

[Henrik Grunell, 2016/02/09 11:47:47]

Acknowledged.

+    BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
+        base::Bind(&WebRTCInternals::ProcessPendingUpdates,
+                   weak_factory_.GetWeakPtr()),
+                   base::TimeDelta::FromMilliseconds(500));
+    pending_updates_queued_ = true;
+  }
 }
 
 void WebRTCInternals::RenderProcessHostDestroyed(RenderProcessHost* host) {
@@ -412,10 +425,10 @@ void WebRTCInternals::OnRendererExit(int render_process_id) {
         record->GetInteger("lid", &lid);
         record->GetInteger("pid", &pid);
 
-        base::DictionaryValue update;
-        update.SetInteger("lid", lid);
-        update.SetInteger("pid", pid);
-        SendUpdate("removePeerConnection", &update);
+        scoped_ptr<base::DictionaryValue> update(new base::DictionaryValue());
+        update->SetInteger("lid", lid);
+        update->SetInteger("pid", pid);
+        SendUpdate("removePeerConnection", std::move(update));
       }
       peer_connection_data_.Remove(i, NULL);
     }
@@ -439,9 +452,9 @@ void WebRTCInternals::OnRendererExit(int render_process_id) {
   }
 
   if (found_any && observers_.might_have_observers()) {
-    base::DictionaryValue update;
-    update.SetInteger("rid", render_process_id);
-    SendUpdate("removeGetUserMediaForRenderer", &update);
+    scoped_ptr<base::DictionaryValue> update(new base::DictionaryValue());
+    update->SetInteger("rid", render_process_id);
+    SendUpdate("removeGetUserMediaForRenderer", std::move(update));
   }
 }
 
@@ -487,4 +500,16 @@ void WebRTCInternals::CreateOrReleasePowerSaveBlocker() {
   }
 }
 
+void WebRTCInternals::ProcessPendingUpdates() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  while (!pending_updates_.empty()) {
+    const auto& update = pending_updates_.front();
+    FOR_EACH_OBSERVER(WebRTCInternalsUIObserver,
+                      observers_,
+                      OnUpdate(update.command(), update.value()));
+    pending_updates_.pop();
+  }
+  pending_updates_queued_ = false;
+}
+
 }  // namespace content