Add explanation text and legal message footer to upload bubble.

chrome/browser/ui/cocoa/autofill/save_card_bubble_view_unittest.mm

 // Copyright 2015 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 "base/json/json_reader.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/values.h"
 #include "chrome/browser/ui/autofill/save_card_bubble_controller.h"
 #import "chrome/browser/ui/cocoa/autofill/save_card_bubble_view_bridge.h"
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 #include "chrome/browser/ui/cocoa/cocoa_profile_test.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #import "ui/events/test/cocoa_test_event_utils.h"
 
 #include <Carbon/Carbon.h>  // For the kVK_* constants.
 
 namespace autofill {
 
 namespace {
 
 class TestSaveCardBubbleController : public SaveCardBubbleController {
  public:
   TestSaveCardBubbleController() {
-    lines_.reset(new LegalMessageLines());
+    std::string message_json =
+        "{"
+        "  \"line\" : ["
+        "    {"
+        "      \"template\": \"Please check out our {0}.\","
+        "      \"template_parameter\": ["
+        "        {"
+        "          \"display_text\": \"terms of service\","
+        "          \"url\": \"http://help.example.com/legal_message\""
+        "        }"
+        "      ]"
+        "    },"
+        "    {"
+        "      \"template\": \"We also have a {0} and {1}.\","
+        "      \"template_parameter\": ["
+        "        {"
+        "          \"display_text\": \"mission statement\","
+        "          \"url\": \"http://www.example.com/our_mission\""
+        "        },"
+        "        {"
+        "          \"display_text\": \"privacy policy\","
+        "          \"url\": \"http://help.example.com/privacy_policy\""
+        "        }"
+        "      ]"
+        "    }"
+        "  ]"
+        "}";
+    scoped_ptr<base::Value> value(base::JSONReader::Read(message_json));
+    base::DictionaryValue* legal_message;
+    value->GetAsDictionary(&legal_message);

[groby-ooo-7-16, 2016/03/03 00:40:23]

ASSERT_TRUE?

[Justin Donnelly, 2016/03/03 18:45:10]

Done. (Required moving this code to a void function.)

+    LegalMessageLine::Parse(*legal_message, &lines_);
 
     on_save_button_was_called_ = false;
     on_cancel_button_was_called_ = false;
     on_learn_more_was_called_ = false;
+    on_legal_message_was_called_ = false;
     on_bubble_closed_was_called_ = false;
   }
 
   // SaveCardBubbleController:
   base::string16 GetWindowTitle() const override { return base::string16(); }
 
   base::string16 GetExplanatoryMessage() const override {
     return base::string16();
   }
 
   const CreditCard GetCard() const override {
     return CreditCard();
   }
 
   void OnSaveButton() override { on_save_button_was_called_ = true; }
   void OnCancelButton() override { on_cancel_button_was_called_ = true; }

[groby-ooo-7-16, 2016/03/03 00:40:23]

It's not the CL to change this, but this looks like it would've been better off
as a mock.

[Justin Donnelly, 2016/03/03 18:45:10]

I don't disagree, but some reviewers are enthusiastically anti-mock so I tend to
use the manual approach when the conditions I care about are relatively simple.
If this gets any more complicated, I'll definitely switch.

[groby-ooo-7-16, 2016/03/07 19:54:58]

Argl. If it's more complicated, the likelihood it should be a mock is smaller :)
We should restrict gmock to fairly trivial objects.

But that's probably a discussion for chromium-dev when we're bored ;)

[Justin Donnelly, 2016/03/07 21:21:02]

I'm not sure there's anything in all of software engineering on which there's
less consensus than mocks. :) I look forward to the discussion.

   void OnLearnMoreClicked() override { on_learn_more_was_called_ = true; }
-  void OnLegalMessageLinkClicked(const GURL& url) override {}
+  void OnLegalMessageLinkClicked(const GURL& url) override {
+    on_legal_message_was_called_ = true;
+    legal_message_url_ = url.spec();
+  }
   void OnBubbleClosed() override { on_bubble_closed_was_called_ = true; }
 
   const LegalMessageLines& GetLegalMessageLines() const override {
-    return *lines_;
+    return lines_;
   }
 
   // Testing state.
   bool on_save_button_was_called() { return on_save_button_was_called_; }
   bool on_cancel_button_was_called() { return on_cancel_button_was_called_; }
   bool on_learn_more_was_called() { return on_learn_more_was_called_; }
+  bool on_legal_message_was_called() { return on_legal_message_was_called_; }
+  std::string legal_message_url() { return legal_message_url_; }
   bool on_bubble_closed_was_called() { return on_bubble_closed_was_called_; }
 
  private:
-  scoped_ptr<LegalMessageLines> lines_;
+  LegalMessageLines lines_;
 
   bool on_save_button_was_called_;
   bool on_cancel_button_was_called_;
   bool on_learn_more_was_called_;
+  bool on_legal_message_was_called_;
+  std::string legal_message_url_;
   bool on_bubble_closed_was_called_;
 };
 
 class SaveCardBubbleViewTest : public CocoaProfileTest {
  public:
   void SetUp() override {
     CocoaProfileTest::SetUp();
     ASSERT_TRUE(browser());
 
     browser_window_controller_ =
@@ skipping 19 matching lines @@
 };
 
 }  // namespace
 
 TEST_F(SaveCardBubbleViewTest, SaveShouldClose) {
   [bridge_->view_controller_ onSaveButton:nil];
 
   EXPECT_TRUE(bubble_controller_->on_save_button_was_called());
   EXPECT_FALSE(bubble_controller_->on_cancel_button_was_called());
   EXPECT_FALSE(bubble_controller_->on_learn_more_was_called());
+  EXPECT_FALSE(bubble_controller_->on_legal_message_was_called());
 
   EXPECT_TRUE(bubble_controller_->on_bubble_closed_was_called());
 }
 
 TEST_F(SaveCardBubbleViewTest, CancelShouldClose) {
   [bridge_->view_controller_ onCancelButton:nil];
 
   EXPECT_FALSE(bubble_controller_->on_save_button_was_called());
   EXPECT_TRUE(bubble_controller_->on_cancel_button_was_called());
   EXPECT_FALSE(bubble_controller_->on_learn_more_was_called());
+  EXPECT_FALSE(bubble_controller_->on_legal_message_was_called());
 
   EXPECT_TRUE(bubble_controller_->on_bubble_closed_was_called());
 }
 
 TEST_F(SaveCardBubbleViewTest, LearnMoreShouldNotClose) {
   NSTextView* textView = nil;
   NSObject* link = nil;
   [bridge_->view_controller_ textView:textView clickedOnLink:link atIndex:0];
 
   EXPECT_FALSE(bubble_controller_->on_save_button_was_called());
   EXPECT_FALSE(bubble_controller_->on_cancel_button_was_called());
   EXPECT_TRUE(bubble_controller_->on_learn_more_was_called());
+  EXPECT_FALSE(bubble_controller_->on_legal_message_was_called());
 
   EXPECT_FALSE(bubble_controller_->on_bubble_closed_was_called());
 }
+
+TEST_F(SaveCardBubbleViewTest, LegalMessageShouldNotClose) {
+  NSString* legalText = @"We also have a mission statement and privacy policy.";
+  base::scoped_nsobject<NSTextView> textView(
+      [[NSTextView alloc] initWithFrame:NSZeroRect]);
+  base::scoped_nsobject<NSAttributedString> attributedMessage(
+      [[NSAttributedString alloc] initWithString:legalText attributes:@{}]);
+  [[textView textStorage] setAttributedString:attributedMessage];
+
+  NSObject* link = nil;
+  [bridge_->view_controller_ textView:textView clickedOnLink:link atIndex:40];
+
+  EXPECT_FALSE(bubble_controller_->on_save_button_was_called());
+  EXPECT_FALSE(bubble_controller_->on_cancel_button_was_called());
+  EXPECT_FALSE(bubble_controller_->on_learn_more_was_called());

[groby-ooo-7-16, 2016/03/03 00:40:23]

Do these EXPECT_FALSE statements matter?

[Justin Donnelly, 2016/03/03 18:45:10]

I think so. Effective unit tests IMO check both that the expected outcome
occurred and that unexpected conditions don't arise.

[groby-ooo-7-16, 2016/03/07 19:54:58]

Meh... There's an infinite number of unexpected conditions.  Can't test for them
all.

[Justin Donnelly, 2016/03/07 21:21:02]

Granted, you can't check them all. But I don't think you should check for none,
either, and I don't think I'm alone in that. Besides, this way, what to check
becomes a subjective question, ideal for bikeshedding. ;)

+  EXPECT_TRUE(bubble_controller_->on_legal_message_was_called());
+
+  std::string url("http://help.example.com/privacy_policy");
+  EXPECT_EQ(url, bubble_controller_->legal_message_url());
+
+  EXPECT_FALSE(bubble_controller_->on_bubble_closed_was_called());
+}
 
 TEST_F(SaveCardBubbleViewTest, ReturnInvokesDefaultAction) {
   [[bridge_->view_controller_ window]
       performKeyEquivalent:cocoa_test_event_utils::KeyEventWithKeyCode(
                                kVK_Return, '\r', NSKeyDown, 0)];
 
   EXPECT_TRUE(bubble_controller_->on_save_button_was_called());
   EXPECT_TRUE(bubble_controller_->on_bubble_closed_was_called());
 }
 
 TEST_F(SaveCardBubbleViewTest, EscapeCloses) {
   [[bridge_->view_controller_ window]
       performKeyEquivalent:cocoa_test_event_utils::KeyEventWithKeyCode(
                                kVK_Escape, '\e', NSKeyDown, 0)];
 
   EXPECT_TRUE(bubble_controller_->on_bubble_closed_was_called());
 }
 
 }  // namespace autofill