Autofill: Add legal message footer to save credit card bubble.

chrome/browser/ui/autofill/save_card_bubble_controller_impl_unittest.cc

+// Copyright 2014 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/ui/autofill/save_card_bubble_controller_impl.h"
+
+#include "base/json/json_reader.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace autofill {
+
+class SaveCardBubbleControllerImplTest
+    : public ChromeRenderViewHostTestHarness {
+ public:
+  SaveCardBubbleControllerImplTest() {}
+
+  void SetUp() override {
+    ChromeRenderViewHostTestHarness::SetUp();
+    SaveCardBubbleControllerImpl::CreateForWebContents(web_contents());
+  }
+
+  void SetLegalMessageExpectSuccess(const std::string& message_json) {
+    scoped_ptr<base::Value> value(base::JSONReader::Read(message_json));
+    const base::ListValue* lines;
+    ASSERT_TRUE(value->GetAsList(&lines));
+    EXPECT_TRUE(controller()->SetLegalMessage(*lines));
+  }
+
+  void SetLegalMessageExpectFailure(const std::string& message_json) {
+    scoped_ptr<base::Value> value(base::JSONReader::Read(message_json));
+    const base::ListValue* lines;
+    ASSERT_TRUE(value->GetAsList(&lines));
+    EXPECT_FALSE(controller()->SetLegalMessage(*lines));
+  }
+
+  // Returns true if lines are the same.
+  bool CompareLegalMessageLines(
+      const SaveCardBubbleController::LegalMessageLine& a,
+      const SaveCardBubbleController::LegalMessageLine& b) {
+    if (a.text != b.text)
+      return false;
+    if (a.links.size() != b.links.size())
+      return false;
+    for (size_t i = 0; i < a.links.size(); ++i) {
+      if (a.links[i].range != b.links[i].range)
+        return false;
+      if (a.links[i].url != b.links[i].url)
+        return false;
+    }
+    return true;
+  }
+
+  // Returns true if messages are the same.
+  bool CompareLegalMessage(

[bondd, 2015/11/13 22:30:44]

AFAIK I can't use operator== on LegalMessageLine without implementing a custom
operator== as part of the struct. Compare is only needed as part of tests, and
I'd like to keep LegalMessageLine as POD, so I added test-only compare functions
here. Is there a better way to do this?

[Evan Stade, 2015/11/14 00:22:50]

sgtm

+      const std::vector<SaveCardBubbleController::LegalMessageLine>& a,
+      const std::vector<SaveCardBubbleController::LegalMessageLine>& b) {
+    if (a.size() != b.size())
+      return false;
+    for (size_t i = 0; i < a.size(); ++i) {
+      if (!CompareLegalMessageLines(a[i], b[i]))
+        return false;
+    }
+    return true;
+  }
+
+ protected:
+  SaveCardBubbleControllerImpl* controller() {
+    return SaveCardBubbleControllerImpl::FromWebContents(web_contents());
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(SaveCardBubbleControllerImplTest);
+};
+
+TEST_F(SaveCardBubbleControllerImplTest, NoParameters) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"This is the entire message.\""
+      "} ]");
+
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text = base::ASCIIToUTF16("This is the entire message.");
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected =
+      std::vector<SaveCardBubbleController::LegalMessageLine>();
+  expected.push_back(expected_line);
+
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, SingleParameter) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"Panda {0}.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bears are fuzzy\","
+      "      \"url\": \"http://www.example.com\""
+      "   } ]"
+      "} ]");
+
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text = base::ASCIIToUTF16("Panda bears are fuzzy.");
+
+  SaveCardBubbleController::LegalMessageLine::Link link;
+  link.range = gfx::Range(6, 21);
+  link.url = GURL("http://www.example.com");
+  expected_line.links.push_back(link);
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, MissingUrl) {
+  SetLegalMessageExpectFailure(
+      "[ {"
+      "   \"template\": \"Panda {0}.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bear\""
+      "   } ]"
+      "} ]");
+  EXPECT_TRUE(CompareLegalMessage(
+      std::vector<SaveCardBubbleController::LegalMessageLine>(),
+      controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, MissingDisplayText) {
+  SetLegalMessageExpectFailure(
+      "[ {"
+      "   \"template\": \"Panda {0}.\","
+      "   \"template_parameter\": [ {"
+      "      \"url\": \"http://www.example.com\""
+      "   } ]"
+      "} ]");
+  EXPECT_TRUE(CompareLegalMessage(
+      std::vector<SaveCardBubbleController::LegalMessageLine>(),
+      controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, EscapeCharacters) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"Panda '{'{0}'}' '{1}' don't $1.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bears\","
+      "      \"url\": \"http://www.example.com\""
+      "   } ]"
+      "} ]");
+
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text = base::ASCIIToUTF16("Panda {bears} {1} don't $1.");
+
+  SaveCardBubbleController::LegalMessageLine::Link link;
+  link.range = gfx::Range(7, 12);
+  link.url = GURL("http://www.example.com");
+  expected_line.links.push_back(link);
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, ConsecutiveDollarSigns) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"$$\""
+      "} ]");
+
+  // Consecutive dollar signs do not expand correctly (see comment in
+  // ReplaceTemplatePlaceholders() in save_card_bubble_controller_impl.cc).
+  // If this is fixed and this test starts to fail, please update the
+  // "Caveats" section of the SaveCardBubbleControllerImpl::SetLegalMessage()
+  // header file comment.
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text = base::ASCIIToUTF16("$$$");
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, DollarAndParenthesis) {
+  // "${" does not expand correctly (see comment in
+  // ReplaceTemplatePlaceholders() in save_card_bubble_controller_impl.cc).
+  // If this is fixed and this test starts to fail, please update the
+  // "Caveats" section of the SaveCardBubbleControllerImpl::SetLegalMessage()
+  // header file comment.
+  SetLegalMessageExpectFailure(
+      "[ {"
+      "   \"template\": \"${0}\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bears\","
+      "      \"url\": \"http://www.example.com\""
+      "   } ]"
+      "} ]");
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, MultipleParameters) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"Panda {0} like {2} eat {1}.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bears\","
+      "      \"url\": \"http://www.example.com/0\""
+      "   }, {"
+      "      \"display_text\": \"bamboo\","
+      "      \"url\": \"http://www.example.com/1\""
+      "   }, {"
+      "      \"display_text\": \"to\","
+      "      \"url\": \"http://www.example.com/2\""
+      "   } ]"
+      "} ]");
+
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text = base::ASCIIToUTF16("Panda bears like to eat bamboo.");
+
+  SaveCardBubbleController::LegalMessageLine::Link link;
+  link.range = gfx::Range(6, 11);
+  link.url = GURL("http://www.example.com/0");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(24, 30);
+  link.url = GURL("http://www.example.com/1");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(17, 19);
+  link.url = GURL("http://www.example.com/2");
+  expected_line.links.push_back(link);
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, MultipleLineElements) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"Panda {0}\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bears\","
+      "      \"url\": \"http://www.example.com/line_0_param_0\""
+      "   } ]"
+      "}, {"
+      "   \"template\": \"like {1} eat {0}.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bamboo\","
+      "      \"url\": \"http://www.example.com/line_1_param_0\""
+      "   }, {"
+      "      \"display_text\": \"to\","
+      "      \"url\": \"http://www.example.com/line_1_param_1\""
+      "   } ]"
+      "}, {"
+      "   \"template\": \"The {0}.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"end\","
+      "      \"url\": \"http://www.example.com/line_2_param_0\""
+      "   } ]"
+      "} ]");
+
+  // Line 0.
+  SaveCardBubbleController::LegalMessageLine expected_line_0;
+  expected_line_0.text = base::ASCIIToUTF16("Panda bears");
+
+  SaveCardBubbleController::LegalMessageLine::Link link;
+  link.range = gfx::Range(6, 11);
+  link.url = GURL("http://www.example.com/line_0_param_0");
+  expected_line_0.links.push_back(link);
+
+  // Line 1.
+  SaveCardBubbleController::LegalMessageLine expected_line_1;
+  expected_line_1.text = base::ASCIIToUTF16("like to eat bamboo.");
+
+  link.range = gfx::Range(12, 18);
+  link.url = GURL("http://www.example.com/line_1_param_0");
+  expected_line_1.links.push_back(link);
+
+  link.range = gfx::Range(5, 7);
+  link.url = GURL("http://www.example.com/line_1_param_1");
+  expected_line_1.links.push_back(link);
+
+  // Line 2.
+  SaveCardBubbleController::LegalMessageLine expected_line_2;
+  expected_line_2.text = base::ASCIIToUTF16("The end.");
+
+  link.range = gfx::Range(4, 7);
+  link.url = GURL("http://www.example.com/line_2_param_0");
+  expected_line_2.links.push_back(link);
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line_0, expected_line_1, expected_line_2};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, EmbeddedNewlines) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"Panda {0}\nlike {2} eat {1}.\nThe {3}.\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"bears\","
+      "      \"url\": \"http://www.example.com/0\""
+      "   }, {"
+      "      \"display_text\": \"bamboo\","
+      "      \"url\": \"http://www.example.com/1\""
+      "   }, {"
+      "      \"display_text\": \"to\","
+      "      \"url\": \"http://www.example.com/2\""
+      "   }, {"
+      "      \"display_text\": \"end\","
+      "      \"url\": \"http://www.example.com/3\""
+      "   } ]"
+      "} ]");
+
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text =
+      base::ASCIIToUTF16("Panda bears\nlike to eat bamboo.\nThe end.");
+
+  SaveCardBubbleController::LegalMessageLine::Link link;
+  link.range = gfx::Range(6, 11);
+  link.url = GURL("http://www.example.com/0");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(24, 30);
+  link.url = GURL("http://www.example.com/1");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(17, 19);
+  link.url = GURL("http://www.example.com/2");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(36, 39);
+  link.url = GURL("http://www.example.com/3");
+  expected_line.links.push_back(link);
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+TEST_F(SaveCardBubbleControllerImplTest, MaximumPlaceholders) {
+  SetLegalMessageExpectSuccess(
+      "[ {"
+      "   \"template\": \"a{0} b{1} c{2} d{3} e{4} f{5} g{6}\","
+      "   \"template_parameter\": [ {"
+      "      \"display_text\": \"A\","
+      "      \"url\": \"http://www.example.com/0\""
+      "   }, {"
+      "      \"display_text\": \"B\","
+      "      \"url\": \"http://www.example.com/1\""
+      "   }, {"
+      "      \"display_text\": \"C\","
+      "      \"url\": \"http://www.example.com/2\""
+      "   }, {"
+      "      \"display_text\": \"D\","
+      "      \"url\": \"http://www.example.com/3\""
+      "   }, {"
+      "      \"display_text\": \"E\","
+      "      \"url\": \"http://www.example.com/4\""
+      "   }, {"
+      "      \"display_text\": \"F\","
+      "      \"url\": \"http://www.example.com/5\""
+      "   }, {"
+      "      \"display_text\": \"G\","
+      "      \"url\": \"http://www.example.com/6\""
+      "   } ]"
+      "} ]");
+
+  SaveCardBubbleController::LegalMessageLine expected_line;
+  expected_line.text = base::ASCIIToUTF16("aA bB cC dD eE fF gG");
+
+  SaveCardBubbleController::LegalMessageLine::Link link;
+  link.range = gfx::Range(1, 2);
+  link.url = GURL("http://www.example.com/0");

[Evan Stade, 2015/11/14 00:22:50]

you can greatly improve the legibility of this test and many others if you use
test case struct arrays. In fact, it would allow you to combine at least some of
these tests into one test.

[bondd, 2015/11/17 00:12:04]

Thanks. I've made this test case and most of the others shorter and clearer by
changing the way the expected output is defined. I considered using test_case
struct arrays to shorten it up more, but I like that the way it is now each test
tests one thing, and that thing is clearly indicated by the test name (e.g.
MaximumPlaceholders or MissingUrl). The testing docs I've read at Google have
always recommended keeping tests as simple and easy to read as possible. I think
the way I've got it now is simple and easy to read. This is subjective though so
your opinion may vary. :)

+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(4, 5);
+  link.url = GURL("http://www.example.com/1");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(7, 8);
+  link.url = GURL("http://www.example.com/2");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(10, 11);
+  link.url = GURL("http://www.example.com/3");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(13, 14);
+  link.url = GURL("http://www.example.com/4");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(16, 17);
+  link.url = GURL("http://www.example.com/5");
+  expected_line.links.push_back(link);
+
+  link.range = gfx::Range(19, 20);
+  link.url = GURL("http://www.example.com/6");
+  expected_line.links.push_back(link);
+
+  std::vector<SaveCardBubbleController::LegalMessageLine> expected = {
+      expected_line};
+  EXPECT_TRUE(
+      CompareLegalMessage(expected, controller()->GetLegalMessageLines()));
+}
+
+}  // namespace autofill