Prevent networkingPrivate.forgetNetwork from removing shared configs

chromeos/dbus/fake_shill_profile_client.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 "chromeos/dbus/fake_shill_profile_client.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/location.h"
 #include "base/memory/ptr_util.h"
@@ skipping 197 matching lines @@
                                            service_properties->DeepCopy());
   return true;
 }
 
 void FakeShillProfileClient::GetProfilePaths(
     std::vector<std::string>* profiles) {
   for (auto iter = profiles_.begin(); iter != profiles_.end(); ++iter)
     profiles->push_back(iter->first);
 }
 
+void FakeShillProfileClient::GetProfilePathsContainingService(
+    const std::string& service_path,
+    std::vector<std::string>* profiles) {
+  for (auto iter = profiles_.begin(); iter != profiles_.end(); ++iter) {
+    std::string profile_path;
+    if (GetServiceDataFromProfile(iter, service_path, &profile_path, nullptr))
+      profiles->push_back(profile_path);
+  }
+}

[stevenjb, 2017/03/29 00:06:45]

blank line

[tbarzic, 2017/03/29 01:48:50]

Done.

 bool FakeShillProfileClient::GetService(const std::string& service_path,
                                         std::string* profile_path,
                                         base::DictionaryValue* properties) {
   properties->Clear();
+
+  auto shared = profiles_.find(GetSharedProfilePath());
+  bool found_shared = false;
+  if (shared != profiles_.end()) {
+    found_shared = GetServiceDataFromProfile(shared, service_path, profile_path,
+                                             properties);
+  }

[stevenjb, 2017/03/29 00:06:45]

So, I kind of hate to ask this, but this is ugly and confusing and I worry about
the next person who comes along and looks at this code.

We should make profiles_ a list instead of a map, and make the path part of
ProfileProperties. It will always be a short list, so iterating or using
std::find() will be fast, and it will simplify this code which should just
iterate through the list and apply the profiles in order (which is what Shill
does IIRC). Then we just need to make sure the shared profile gets added first.

[tbarzic, 2017/03/29 01:48:50]

Sounds reasonable. Done.

+
   for (auto iter = profiles_.begin(); iter != profiles_.end(); ++iter) {
-    const ProfileProperties* profile = iter->second.get();
-    const base::DictionaryValue* entry;
-    if (!profile->entries.GetDictionaryWithoutPathExpansion(
-            service_path, &entry)) {
+    // Shared profile properties have already been applied before the for loop.
+    if (iter->first == GetSharedProfilePath())
       continue;
+    if (GetServiceDataFromProfile(iter, service_path, profile_path,
+                                  properties)) {
+      return true;
     }
-    *profile_path = iter->first;
-    properties->MergeDictionary(entry);
-    return true;
   }
-  return false;
+  return found_shared;
 }
 
 void FakeShillProfileClient::ClearProfiles() {
   profiles_.clear();
 }
 
 FakeShillProfileClient::ProfileProperties* FakeShillProfileClient::GetProfile(
     const dbus::ObjectPath& profile_path,
     const ErrorCallback& error_callback) {
   auto found = profiles_.find(profile_path.value());
   if (found == profiles_.end()) {
     if (!error_callback.is_null())
       error_callback.Run("Error.InvalidProfile", "Invalid profile");
     return nullptr;
   }
 
   return found->second.get();
 }
 
+bool FakeShillProfileClient::GetServiceDataFromProfile(
+    const ProfileMap::iterator& iter,

[stevenjb, 2017/03/29 00:06:45]

I would rather pass the profile path and const ProfileProperties* separately
than pass an iterator.

[tbarzic, 2017/03/29 01:48:50]

Done in sense that now only ProfileProperties are needed.

+    const std::string& service_path,
+    std::string* profile_path,
+    base::DictionaryValue* properties) {
+  const ProfileProperties* profile = iter->second.get();
+  const base::DictionaryValue* entry;
+  if (!profile->entries.GetDictionaryWithoutPathExpansion(service_path,
+                                                          &entry)) {
+    return false;
+  }
+  *profile_path = iter->first;
+  if (properties)
+    properties->MergeDictionary(entry);
+  return true;
+}
+
 }  // namespace chromeos