Remove more usage of the base::ListValue::Append(Value*) overload.

content/browser/webrtc/webrtc_internals.cc

 // 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.
 
 #include "content/browser/webrtc/webrtc_internals.h"
 
 #include <stddef.h>
 
+#include <memory>
+#include <utility>
+
 #include "base/strings/string_number_conversions.h"
 #include "build/build_config.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/web_contents/web_contents_view.h"
 #include "content/browser/webrtc/webrtc_internals_ui_observer.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/web_contents.h"
 #include "device/power_save_blocker/power_save_blocker.h"
 #include "ipc/ipc_platform_file.h"
@@ skipping 91 matching lines @@
 }
 
 void WebRTCInternals::OnAddPeerConnection(int render_process_id,
                                           ProcessId pid,
                                           int lid,
                                           const string& url,
                                           const string& rtc_configuration,
                                           const string& constraints) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::DictionaryValue* dict = new base::DictionaryValue();
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->SetInteger("rid", render_process_id);
   dict->SetInteger("pid", static_cast<int>(pid));
   dict->SetInteger("lid", lid);
   dict->SetString("rtcConfiguration", rtc_configuration);
   dict->SetString("constraints", constraints);
   dict->SetString("url", url);
-  peer_connection_data_.Append(dict);
-  CreateOrReleasePowerSaveBlocker();
 
   if (observers_.might_have_observers())
     SendUpdate("addPeerConnection", dict->CreateDeepCopy());
 
+  peer_connection_data_.Append(std::move(dict));

[dcheng, 2016/08/30 06:47:48]

I reordered this, but I'm happy to fix this another way as well. The observer
call is just queues some JS to execute, so I figure this is fine.

(Alternately, we /could/ use WrapUnique but it's nice to avoid it when possible.
We could just always call CreateDeepCopy when adding it to the connection data,
but that seems "wasteful")

[danakj, 2016/09/06 21:55:39]

Hm.. or just save a raw pointer before doing the move. I'm not so sure about
reordering, as I don't know what contracts would be expected from these APIs. I
think you should either preserve ordering or get a review from owners who know
best here.

[dcheng, 2016/09/06 22:01:17]

I'll ask a webrtc owner to review.

It seems pretty sketchy either way, and I think I'd prefer less use of raw
pointers over more.

+  CreateOrReleasePowerSaveBlocker();
+
   if (render_process_id_set_.insert(render_process_id).second) {
     RenderProcessHost* host = RenderProcessHost::FromID(render_process_id);
     if (host)
       host->AddObserver(this);
   }
 }
 
 void WebRTCInternals::OnRemovePeerConnection(ProcessId pid, int lid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   for (size_t i = 0; i < peer_connection_data_.GetSize(); ++i) {
@@ skipping 34 matching lines @@
     record->GetInteger("lid", &this_lid);
 
     if (this_pid != static_cast<int>(pid) || this_lid != lid)
       continue;
 
     // Append the update to the end of the log.
     base::ListValue* log = EnsureLogList(record);
     if (!log)
       return;
 
-    base::DictionaryValue* log_entry = new base::DictionaryValue();
-    if (!log_entry)
-      return;
+    std::unique_ptr<base::DictionaryValue> log_entry(
+        new base::DictionaryValue());
 
     double epoch_time = base::Time::Now().ToJsTime();
     string time = base::DoubleToString(epoch_time);
     log_entry->SetString("time", time);
     log_entry->SetString("type", type);
     log_entry->SetString("value", value);
-    log->Append(log_entry);
 
     if (observers_.might_have_observers()) {
       std::unique_ptr<base::DictionaryValue> update(
           new base::DictionaryValue());
       update->SetInteger("pid", static_cast<int>(pid));
       update->SetInteger("lid", lid);
-      update->MergeDictionary(log_entry);
+      update->MergeDictionary(log_entry.get());
 
       SendUpdate("updatePeerConnection", std::move(update));
     }
+
+    log->Append(std::move(log_entry));
+
     return;
   }
 }
 
 void WebRTCInternals::OnAddStats(base::ProcessId pid, int lid,
                                  const base::ListValue& value) {
   if (!observers_.might_have_observers())
     return;
 
   std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->SetInteger("pid", static_cast<int>(pid));
   dict->SetInteger("lid", lid);
 
   dict->Set("reports", value.CreateDeepCopy());
 
   SendUpdate("addStats", std::move(dict));
 }
 
 void WebRTCInternals::OnGetUserMedia(int rid,
                                      base::ProcessId pid,
                                      const std::string& origin,
                                      bool audio,
                                      bool video,
                                      const std::string& audio_constraints,
                                      const std::string& video_constraints) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  base::DictionaryValue* dict = new base::DictionaryValue();
+  std::unique_ptr<base::DictionaryValue> dict(new base::DictionaryValue());
   dict->SetInteger("rid", rid);
   dict->SetInteger("pid", static_cast<int>(pid));
   dict->SetString("origin", origin);
   if (audio)
     dict->SetString("audio", audio_constraints);
   if (video)
     dict->SetString("video", video_constraints);
 
-  get_user_media_requests_.Append(dict);
+  get_user_media_requests_.Append(std::move(dict));
 
   if (observers_.might_have_observers())
     SendUpdate("addGetUserMedia", dict->CreateDeepCopy());
 
   if (render_process_id_set_.insert(rid).second) {
     RenderProcessHost* host = RenderProcessHost::FromID(rid);
     if (host)
       host->AddObserver(this);
   }
 }
@@ skipping 269 matching lines @@
   while (!pending_updates_.empty()) {
     const auto& update = pending_updates_.front();
     FOR_EACH_OBSERVER(WebRTCInternalsUIObserver,
                       observers_,
                       OnUpdate(update.command(), update.value()));
     pending_updates_.pop();
   }
 }
 
 }  // namespace content