Views::Textfield: Prevent revealing password text.

Currently, the Yank command on Mac and the selection clipboard on Linux can
reveal the text of an obscured/password Views::Textfield. This CL modifies
Textfield::UpdateSelectionClipboard() to ensure that the selection clipboard is
not modified for a password textfield. Textfield::ExecuteTextEditCommand(..) is
also modified to only update the yank kill buffer for a non-password textfield.
Unit tests which demonstrate the problem are also added.

BUG=648511, 648509

Review-Url: https://codereview.chromium.org/2358463002
Cr-Commit-Position: refs/heads/master@{#419977}

ui/views/controls/textfield/textfield.cc

@@ -1582,7 +1582,7 @@ void Textfield::ExecuteTextEditCommand(ui::TextEditCommand command) {
     case ui::TextEditCommand::DELETE_TO_BEGINNING_OF_PARAGRAPH:
     case ui::TextEditCommand::DELETE_TO_END_OF_LINE:
     case ui::TextEditCommand::DELETE_TO_END_OF_PARAGRAPH:
-      add_to_kill_buffer = true;
+      add_to_kill_buffer = text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD;
     // Fall through.
     case ui::TextEditCommand::DELETE_WORD_BACKWARD:
     case ui::TextEditCommand::DELETE_WORD_FORWARD:
@@ -2015,7 +2015,8 @@ void Textfield::CreateTouchSelectionControllerAndNotifyIt() {
 
 void Textfield::UpdateSelectionClipboard() const {
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
-  if (performing_user_action_ && HasSelection()) {
+  if (performing_user_action_ && HasSelection() &&
+      text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD) {
     ui::ScopedClipboardWriter(
         ui::CLIPBOARD_TYPE_SELECTION).WriteText(GetSelectedText());
     if (controller_)

ui/views/controls/textfield/textfield.h

@@ -73,7 +73,8 @@ class VIEWS_EXPORT Textfield : public View,
   // features. The flags is the bit map of ui::TextInputFlags.
   void SetTextInputFlags(int flags);
 
-  // Gets the text currently displayed in the Textfield.
+  // Gets the text for the Textfield. Call sites should take care to not reveal
+  // the text for a password textfield.
   const base::string16& text() const { return model_->text(); }
 
   // Sets the text currently displayed in the Textfield.  This doesn't
@@ -88,7 +89,8 @@ class VIEWS_EXPORT Textfield : public View,
   // Inserts |new_text| at the cursor position, replacing any selected text.
   void InsertOrReplaceText(const base::string16& new_text);
 
-  // Returns the text that is currently selected.
+  // Returns the text that is currently selected. Call sites should take care to
+  // not reveal the text for a password textfield.
   base::string16 GetSelectedText() const;
 
   // Select the entire text range. If |reversed| is true, the range will end at
@@ -386,7 +388,8 @@ class VIEWS_EXPORT Textfield : public View,
 
   void CreateTouchSelectionControllerAndNotifyIt();
 
-  // Updates the selection clipboard to any non-empty text selection.
+  // Updates the selection clipboard to any non-empty text selection for a non-
+  // password textfield.
   void UpdateSelectionClipboard() const;
 
   // Pastes the selection clipboard for the specified mouse event.

ui/views/controls/textfield/textfield_model.cc

@@ -344,11 +344,15 @@ void TextfieldModel::Append(const base::string16& new_text) {
 }
 
 bool TextfieldModel::Delete(bool add_to_kill_buffer) {
+  // |add_to_kill_buffer| should never be true for an obscured textfield.
+  DCHECK(!add_to_kill_buffer || !render_text_->obscured());
+
   if (HasCompositionText()) {
     // No undo/redo for composition text.
     CancelCompositionText();
     return true;
   }
+
   if (HasSelection()) {
     if (add_to_kill_buffer)
       SetKillBuffer(GetSelectedText());
@@ -369,11 +373,15 @@ bool TextfieldModel::Delete(bool add_to_kill_buffer) {
 }
 
 bool TextfieldModel::Backspace(bool add_to_kill_buffer) {
+  // |add_to_kill_buffer| should never be true for an obscured textfield.
+  DCHECK(!add_to_kill_buffer || !render_text_->obscured());
+
   if (HasCompositionText()) {
     // No undo/redo for composition text.
     CancelCompositionText();
     return true;
   }
+
   if (HasSelection()) {
     if (add_to_kill_buffer)
       SetKillBuffer(GetSelectedText());
@@ -430,8 +438,7 @@ bool TextfieldModel::MoveCursorTo(const gfx::Point& point, bool select) {
 }
 
 base::string16 TextfieldModel::GetSelectedText() const {
-  return text().substr(render_text_->selection().GetMin(),
-                       render_text_->selection().length());
+  return GetTextFromRange(render_text_->selection());
 }
 
 void TextfieldModel::SelectRange(const gfx::Range& range) {

ui/views/controls/textfield/textfield_unittest.cc

@@ -2081,10 +2081,23 @@ TEST_F(TextfieldTest, Yank) {
   Textfield* textfield2 = static_cast<Textfield*>(GetFocusedView());
   EXPECT_TRUE(textfield2->text().empty());
 
-  // Verify yanked text persists across multiple textfields.
+  // Verify yanked text persists across multiple textfields and that yanking
+  // into a password textfield works.
+  textfield2->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
   SendKeyPress(ui::VKEY_Y, ui::EF_CONTROL_DOWN);
   EXPECT_STR_EQ("ef", textfield2->text());
   EXPECT_EQ(gfx::Range(2), textfield2->GetSelectedRange());
+
+  // Verify deletion in a password textfield does not modify the kill buffer.
+  textfield2->SetText(ASCIIToUTF16("hello"));
+  textfield2->SelectRange(gfx::Range(0));
+  SendKeyPress(ui::VKEY_K, ui::EF_CONTROL_DOWN);
+  EXPECT_TRUE(textfield2->text().empty());
+
+  textfield_->RequestFocus();
+  textfield_->SelectRange(gfx::Range(0));
+  SendKeyPress(ui::VKEY_Y, ui::EF_CONTROL_DOWN);
+  EXPECT_STR_EQ("efabefeef", textfield_->text());
 }
 
 #endif  // defined(OS_MACOSX)
@@ -2720,6 +2733,46 @@ TEST_F(TextfieldTest, DISABLED_SelectionClipboard) {
   EXPECT_STR_EQ("other", GetClipboardText(ui::CLIPBOARD_TYPE_SELECTION));
   EXPECT_EQ(ui::CLIPBOARD_TYPE_LAST, GetAndResetCopiedToClipboard());
 }
+
+// Verify that the selection clipboard is not updated for selections on a
+// password textfield. Disabled due to http://crbug.com/396477.
+TEST_F(TextfieldTest, DISABLED_SelectionClipboard_Password) {
+  InitTextfields(2);
+  textfield_->SetText(ASCIIToUTF16("abcd"));
+
+  // Select-all should update the selection clipboard for a non-password
+  // textfield.
+  SendKeyEvent(ui::VKEY_A, false, true);
+  EXPECT_EQ(gfx::Range(0, 4), textfield_->GetSelectedRange());
+  EXPECT_STR_EQ("abcd", GetClipboardText(ui::CLIPBOARD_TYPE_SELECTION));
+  EXPECT_EQ(ui::CLIPBOARD_TYPE_SELECTION, GetAndResetCopiedToClipboard());
+
+  // Move focus to the next textfield.
+  widget_->GetFocusManager()->AdvanceFocus(false);
+  EXPECT_EQ(2, GetFocusedView()->id());
+  Textfield* textfield2 = static_cast<Textfield*>(GetFocusedView());
+
+  // Select-all should not modify the selection clipboard for a password
+  // textfield.
+  textfield2->SetText(ASCIIToUTF16("1234"));
+  textfield2->SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
+  SendKeyEvent(ui::VKEY_A, false, true);
+  EXPECT_EQ(gfx::Range(0, 4), textfield2->GetSelectedRange());
+  EXPECT_STR_EQ("abcd", GetClipboardText(ui::CLIPBOARD_TYPE_SELECTION));
+  EXPECT_EQ(ui::CLIPBOARD_TYPE_LAST, GetAndResetCopiedToClipboard());
+
+  // Shift-Left/Right should not modify the selection clipboard for a password
+  // textfield.
+  SendKeyEvent(ui::VKEY_LEFT, true, false);
+  EXPECT_EQ(gfx::Range(0, 3), textfield2->GetSelectedRange());
+  EXPECT_STR_EQ("abcd", GetClipboardText(ui::CLIPBOARD_TYPE_SELECTION));
+  EXPECT_EQ(ui::CLIPBOARD_TYPE_LAST, GetAndResetCopiedToClipboard());
+
+  SendKeyEvent(ui::VKEY_RIGHT, true, false);
+  EXPECT_EQ(gfx::Range(0, 4), textfield2->GetSelectedRange());
+  EXPECT_STR_EQ("abcd", GetClipboardText(ui::CLIPBOARD_TYPE_SELECTION));
+  EXPECT_EQ(ui::CLIPBOARD_TYPE_LAST, GetAndResetCopiedToClipboard());
+}
 #endif
 
 // Long_Press gesture in Textfield can initiate a drag and drop now.