Disallow calling value builders' Set/Append(??, bool) methods.

Currently calling DictionaryValue::Set(??, bool) and
ListValue::Append(bool) would result in calling int variant from
the overloaded methods(!). This is confusing and can be a source
of bugs.
(The current suggested way is to explicitly call SetBoolean and
AppendBoolean).

There is only once place where this was happening, in
extension_printer_handler.cc. Fix that code to use boolean variant,
by using SetBoolean() method.

This CL will also let us overload the bool variant in future. This CL
- Provides an explicit "const char*" param overload.
- Declares a bool overload but makes it inaccessible (= delete).

This will
- Make sure attempting to call DictionaryValue::Set with bool
  will fail compile (call to deleted member function 'Set'). So
  accidental introduction of such bugs won't be possible.
- The char* variant will make sure we don't attempt to resolve
  bool to this pointer variant.
- Give us the ability to remove GetBoolean/AppendBoolean.

Bug: 831839
Change-Id: If61ce8a44799ff8e12957d5206baf74467655894
Test: Expect no behavior change, touches webui/print_preview FYI
Reviewed-on: https://chromium-review.googlesource.com/1008870Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550196}

chrome/browser/ui/webui/print_preview/extension_printer_handler.cc

@@ -373,7 +373,7 @@ void ExtensionPrinterHandler::OnUsbDevicesEnumerated(
                          device->product_string(), base::string16(), false))
                 .Set("extensionId", extension->id())
                 .Set("extensionName", extension->name())
-                .Set("provisional", true)
+                .SetBoolean("provisional", true)
                 .Build());
       }
     }

chrome/browser/ui/webui/print_preview/extension_printer_handler_unittest.cc

@@ -595,7 +595,7 @@ TEST_F(ExtensionPrinterHandlerTest, GetUsbPrinters) {
           .Set("name", "USB Printer")
           .Set("extensionName", "Provider 1")
           .Set("extensionId", extension_1->id())
-          .Set("provisional", true)
+          .SetBoolean("provisional", true)
           .Build());
   std::unique_ptr<base::DictionaryValue> extension_2_entry(
       DictionaryBuilder()
@@ -605,7 +605,7 @@ TEST_F(ExtensionPrinterHandlerTest, GetUsbPrinters) {
           .Set("name", "USB Printer")
           .Set("extensionName", "Provider 2")
           .Set("extensionId", extension_2->id())
-          .Set("provisional", true)
+          .SetBoolean("provisional", true)
           .Build());
   EXPECT_TRUE(printers->Find(*extension_1_entry) != printers->end());
   EXPECT_TRUE(printers->Find(*extension_2_entry) != printers->end());

extensions/common/value_builder.cc

@@ -27,6 +27,11 @@ std::string DictionaryBuilder::ToJSON() const {
   return json;
 }
 
+DictionaryBuilder& DictionaryBuilder::Set(const std::string& path,
+                                          const char* in_value) {
+  return Set(path, std::string(in_value));
+}
+
 DictionaryBuilder& DictionaryBuilder::Set(const std::string& path,
                                           int in_value) {
   dict_->SetWithoutPathExpansion(path, std::make_unique<base::Value>(in_value));
@@ -81,6 +86,10 @@ ListBuilder& ListBuilder::Append(double in_value) {
   return *this;
 }
 
+ListBuilder& ListBuilder::Append(const char* in_value) {
+  return Append(std::string(in_value));
+}
+
 ListBuilder& ListBuilder::Append(const std::string& in_value) {
   list_->AppendString(in_value);
   return *this;

extensions/common/value_builder.h

@@ -53,14 +53,21 @@ class DictionaryBuilder {
   // times as you like.
   std::string ToJSON() const;
 
+  // TODO(lazyboy): Switch |path| to base::StringPiece, just like base::Value's
+  // parameters.
   DictionaryBuilder& Set(const std::string& path, int in_value);
   DictionaryBuilder& Set(const std::string& path, double in_value);
+  DictionaryBuilder& Set(const std::string& path, const char* in_value);
   DictionaryBuilder& Set(const std::string& path, const std::string& in_value);
   DictionaryBuilder& Set(const std::string& path,
                          const base::string16& in_value);
   DictionaryBuilder& Set(const std::string& path,
                          std::unique_ptr<base::Value> in_value);
 
+  // WARNING: This is not defined (intentional). Linking will fail. Use
+  // SetBoolean for now.
+  // TODO(lazyboy): Define this and remove SetBoolean().
+  DictionaryBuilder& Set(const std::string& path, bool in_value) = delete;
   // Named differently because overload resolution is too eager to
   // convert implicitly to bool.
   DictionaryBuilder& SetBoolean(const std::string& path, bool in_value);
@@ -80,10 +87,15 @@ class ListBuilder {
 
   ListBuilder& Append(int in_value);
   ListBuilder& Append(double in_value);
+  ListBuilder& Append(const char* in_value);
   ListBuilder& Append(const std::string& in_value);
   ListBuilder& Append(const base::string16& in_value);
   ListBuilder& Append(std::unique_ptr<base::Value> in_value);
 
+  // WARNING: This is not defined (intentional). Linking will fail. Use
+  // AppendBoolean for now.
+  // TODO(lazyboy): Define this and remove AppendBoolean().
+  ListBuilder& Append(bool in_value) = delete;
   // Named differently because overload resolution is too eager to
   // convert implicitly to bool.
   ListBuilder& AppendBoolean(bool in_value);