Make extensions DSE persistent in browser prefs

chrome/browser/search_engines/template_url_service_test_util.h

Index: chrome/browser/search_engines/template_url_service_test_util.h
diff --git a/chrome/browser/search_engines/template_url_service_test_util.h b/chrome/browser/search_engines/template_url_service_test_util.h
index 566fb9215b536f1128a7ca0eb5b14ec9713568fa..bda85fc7903d8c552d6e3358b216f2304e5e094b 100644
--- a/chrome/browser/search_engines/template_url_service_test_util.h
+++ b/chrome/browser/search_engines/template_url_service_test_util.h
@@ -12,6 +12,7 @@
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/strings/string16.h"
+#include "components/search_engines/template_url.h"
 #include "components/search_engines/template_url_data.h"
 #include "components/search_engines/template_url_service_observer.h"
 
@@ -65,6 +66,22 @@ class TemplateURLServiceTestUtil : public TemplateURLServiceObserver {
   // Sets the google base url.  |base_url| must be valid.
   void SetGoogleBaseURL(const GURL& base_url);
 
+  // Write default search engine into extension controlled preference in test

[Peter Kasting, 2016/12/22 20:49:03]

Nit: Function comments should be declarative ("writes") rather than imperative
("write"); see
http://google.github.io/styleguide/cppguide.html#Function_Comments .  (4 places)

[Alexander Yashkin, 2016/12/23 19:44:08]

Done

+  // profile.
+  void SetExtensionDefaultSearchInPrefs(const std::string& ext_id,

[Peter Kasting, 2016/12/22 20:49:03]

This first arg is not actually used in the implementation.  Remove unused
parameters.  (2 places)

[Alexander Yashkin, 2016/12/23 19:44:08]

This remains from previous approach, removed.

+                                        const TemplateURLData& ext_data);

[Peter Kasting, 2016/12/22 20:49:03]

Nit: Avoid abbreviations like "ext" (could read as "external" or something); see
http://google.github.io/styleguide/cppguide.html#General_Naming_Rules .  If it's
clear that this is the extension ID and data, just say |id| and |data|.  If not,
say |extension_id| or the like.  (Multiple places)

[Alexander Yashkin, 2016/12/23 19:44:08]

Done, replaced with "extension_" or removed.

+  // Remove extension controlled default search engine preference from test

[Peter Kasting, 2016/12/22 20:49:03]

Nit: I'd still put a blink line above this comment even though these two
functions are loosely-related; it's enough to order them next to each other.  (2
places)

[Alexander Yashkin, 2016/12/23 19:44:08]

Done

+  // profile.
+  void RemoveExtensionDefaultSearchFromPrefs(const std::string& ext_id);
+
+  // Add extension turl to model and override default search preference if
+  // extension wants to be default.
+  TemplateURL* AddExtensionControlledTURL(
+      std::unique_ptr<TemplateURL> ext_dse,
+      std::unique_ptr<TemplateURL::AssociatedExtensionInfo> ext_info);
+  // Remove extension turl from preferences and model.

[Peter Kasting, 2016/12/22 20:49:03]

Nit: Expand this (and comments like it) to cover the function contract and give
more detail where relevant.  For example:

Removes a TemplateURL controlled by |ext_id| from the model, and, if necessary,
from the extension-controlled default search preference.  This TemplateURL must
exist.

[Alexander Yashkin, 2016/12/23 19:44:08]

Done, thanks.

+  void RemoveExtensionControlledTURL(const std::string& ext_id);
+
   KeywordWebDataService* web_data_service() { return web_data_service_.get(); }
   TemplateURLService* model() { return model_.get(); }
   TestingProfile* profile() { return profile_.get(); }