Remove deprecated ListValue::Append(Value*) overload on Linux.

chrome/browser/devtools/devtools_targets_ui.cc

 // Copyright 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 "chrome/browser/devtools/devtools_targets_ui.h"
 
+#include <memory>
 #include <utility>
 
 #include "base/location.h"
+#include "base/memory/ptr_util.h"
 #include "base/memory/weak_ptr.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "base/version.h"
 #include "chrome/browser/devtools/device/devtools_android_bridge.h"
 #include "content/public/browser/browser_child_process_observer.h"
@@ skipping 206 matching lines @@
           base::TimeDelta::FromMilliseconds(kUpdateDelay)));
 }
 
 void LocalTargetsUIHandler::UpdateTargets() {
   SendTargets(DevToolsAgentHost::GetOrCreateAll());
 }
 
 void LocalTargetsUIHandler::SendTargets(
     const content::DevToolsAgentHost::List& targets) {
   base::ListValue list_value;
-  std::map<std::string, base::DictionaryValue*> id_to_descriptor;
+  std::map<std::string, std::unique_ptr<base::DictionaryValue>>
+      id_to_descriptor;
 
   targets_.clear();
   for (scoped_refptr<DevToolsAgentHost> host : targets) {
     targets_[host->GetId()] = host;
     id_to_descriptor[host->GetId()] = Serialize(host);
   }
 
   for (auto& it : targets_) {
     scoped_refptr<DevToolsAgentHost> host = it.second;
-    base::DictionaryValue* descriptor = id_to_descriptor[host->GetId()];
+    std::unique_ptr<base::DictionaryValue> descriptor =
+        std::move(id_to_descriptor[host->GetId()]);
+    DCHECK(descriptor);
     std::string parent_id = host->GetParentId();
     if (parent_id.empty() || id_to_descriptor.count(parent_id) == 0) {
-      list_value.Append(descriptor);
+      list_value.Append(std::move(descriptor));
     } else {
-      base::DictionaryValue* parent = id_to_descriptor[parent_id];
+      base::DictionaryValue* parent = id_to_descriptor[parent_id].get();

[danakj, 2016/09/15 21:07:34]

Um, hm. We're moving things out of this map now, which means we leave behind
null. Is it true that we'll always still have parent_id's uniqueptr in the set
here? Cuz L249 removes them..

[dcheng, 2016/09/15 21:10:22]

Well. The tests pass ^_^

But I agree that this is very confusing, and frankly, I'm not sure it's right.

[pfeldman, 2016/09/16 21:39:02]

Haven't looked at this code for a while...

So we have a list and we want to end up with the tree based on the parentId
relationship. It means that you need to:
- maintain the map with ownership in one id_to_descriptor.
- keep rawpointer-based mapping in another map next to it for parent lookup.

Otherwise line 258 fails with nullptr access. And there are obviously no tests
for this branch :(

[dcheng, 2016/09/16 23:15:01]

Keeping two trees seems strictly worse... so I just turned it back into raw
pointers.

       base::ListValue* guests = NULL;
       if (!parent->GetList(kGuestList, &guests)) {
         guests = new base::ListValue();
         parent->Set(kGuestList, guests);
       }
-      guests->Append(descriptor);
+      guests->Append(std::move(descriptor));
     }
   }
 
   SendSerializedTargets(list_value);
 }
 
 // AdbTargetsUIHandler --------------------------------------------------------
 
 class AdbTargetsUIHandler
     : public DevToolsTargetsUIHandler,
@@ skipping 94 matching lines @@
       std::string browser_id = browser->GetId();
       browser_data->SetString(kTargetIdField, browser_id);
       browser_data->SetString(kTargetSourceField, source_id());
 
       base::ListValue* page_list = new base::ListValue();
       remote_browsers_[browser_id] = browser;
             browser_data->Set(kAdbPagesList, page_list);
       for (const auto& page : browser->pages()) {
         scoped_refptr<DevToolsAgentHost> host =
             android_bridge_->CreatePageTarget(page);
-        base::DictionaryValue* target_data = Serialize(host);
+        std::unique_ptr<base::DictionaryValue> target_data = Serialize(host);
         target_data->SetBoolean(
             kAdbAttachedForeignField,
             host->IsAttached() &&
                 !android_bridge_->HasDevToolsWindow(host->GetId()));
         // Pass the screen size in the target object to make sure that
         // the caching logic does not prevent the target item from updating
         // when the screen size changes.
         gfx::Size screen_size = device->screen_size();
         target_data->SetInteger(kAdbScreenWidthField, screen_size.width());
         target_data->SetInteger(kAdbScreenHeightField, screen_size.height());
         targets_[host->GetId()] = host;
-        page_list->Append(target_data);
+        page_list->Append(std::move(target_data));
       }
       browser_list->Append(std::move(browser_data));
     }
 
     device_list.Append(std::move(device_data));
   }
   SendSerializedTargets(device_list);
 }
 
 } // namespace
@@ skipping 37 matching lines @@
 
 void DevToolsTargetsUIHandler::Open(const std::string& browser_id,
                                     const std::string& url) {
 }
 
 scoped_refptr<DevToolsAgentHost>
 DevToolsTargetsUIHandler::GetBrowserAgentHost(const std::string& browser_id) {
   return NULL;
 }
 
-base::DictionaryValue* DevToolsTargetsUIHandler::Serialize(
+std::unique_ptr<base::DictionaryValue> DevToolsTargetsUIHandler::Serialize(
     scoped_refptr<DevToolsAgentHost> host) {
-  base::DictionaryValue* target_data = new base::DictionaryValue();
+  auto target_data = base::MakeUnique<base::DictionaryValue>();
   target_data->SetString(kTargetSourceField, source_id_);
   target_data->SetString(kTargetIdField, host->GetId());
   target_data->SetString(kTargetTypeField, host->GetType());
   target_data->SetBoolean(kAttachedField, host->IsAttached());
   target_data->SetString(kUrlField, host->GetURL().spec());
   target_data->SetString(kNameField, host->GetTitle());
   target_data->SetString(kFaviconUrlField, host->GetFaviconURL().spec());
   target_data->SetString(kDescriptionField, host->GetDescription());
   return target_data;
 }
@@ skipping 42 matching lines @@
     device_status_dict->SetString(kPortForwardingBrowserId,
                                   sit->first->GetId());
 
     std::string device_id = base::StringPrintf(
         kAdbDeviceIdFormat,
         sit->first->serial().c_str());
     result.Set(device_id, device_status_dict);
   }
   callback_.Run(result);
 }