Views Textfield fixes and cleanup.

Views Textfield fixes and cleanup.

Using STYLE_OBSCURED in the ctor broke with r244436.
Remove 'styles', that ctor, and IsObscured/SetObscured.
Use TextInputType for password textfield configuration.
(rename textfield 'obscured' identifiers to 'password')

Inline STYLE_LOWERCASE to the EditSearchEngineDialog.
Remove GetTextForDisplay and its related handling.

Inline set_controller; remove GetController.
Cleanup users and unit tests.

BUG=131660, 334252
TEST=Password textfields work (show '*'s, reveal last typed char, toggle in wifi/wimax dialogs, etc.) as expected. The EditSearchEngineDialog's keyword textfield holds lowercase text as expected.
R=sky@chromium.org
TBR=pfeldman@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=245243

[msw, 2014-01-14 22:34:07]

Hey Scott, please take a look; thanks!

[sky, 2014-01-14 23:17:12]

https://codereview.chromium.org/138363004/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/edit_search_engine_dialog.cc (right):

https://codereview.chromium.org/138363004/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/edit_search_engine_dialog.cc:110:
sender->SelectSelectionModel(selection_model);
This seems really heavyweight and easy to lose state. For example, what about if
not all the text is visible. Does doing this force scrolling to a certain
region?

[msw, 2014-01-15 01:22:02]

https://codereview.chromium.org/138363004/diff/40001/chrome/browser/ui/views/...
File chrome/browser/ui/views/edit_search_engine_dialog.cc (right):

https://codereview.chromium.org/138363004/diff/40001/chrome/browser/ui/views/...
chrome/browser/ui/views/edit_search_engine_dialog.cc:110:
sender->SelectSelectionModel(selection_model);
On 2014/01/14 23:17:13, sky wrote:
> This seems really heavyweight and easy to lose state. For example, what about
if
> not all the text is visible. Does doing this force scrolling to a certain
> region?

The ToT implementation is worse, it actually clamps the cursor to the end of the
input after entering an uppercase value anywhere in the string, and leaves it
there! Restoring the selection model is actually an improvement. This specific
patch set *did* break IME input here, but the new patch set fixes that issue.

But yes, setting a cursor/selection does indeed scroll to show the cursor (or
the selection range's end), and calling SetText forces the cursor to the end of
the textfield. So, it may scroll jarringly when you start typing in the middle
of a long string. I added a TODO to address this pre-existing defect later.

[sky, 2014-01-15 14:12:29]

LGTM

[msw, 2014-01-15 17:45:08]

+TBR pfeldman for content/shell/browser/shell_views.cc

[commit-bot: I haz the power, 2014-01-15 17:47:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/138363004/160001

[commit-bot: I haz the power, 2014-01-15 22:24:57]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2014-01-15 22:53:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/138363004/160001

[commit-bot: I haz the power, 2014-01-16 06:31:57]

Failed to apply patch for ui/views/controls/textfield/textfield.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file ui/views/controls/textfield/textfield.cc
  Hunk #4 FAILED at 102.
  Hunk #15 succeeded at 1426 (offset 2 lines).
  Hunk #16 succeeded at 1436 (offset 2 lines).
  Hunk #17 succeeded at 1445 (offset 2 lines).
  Hunk #18 succeeded at 1494 (offset 2 lines).
  1 out of 18 hunks FAILED -- saving rejects to file
ui/views/controls/textfield/textfield.cc.rej

Patch:       ui/views/controls/textfield/textfield.cc
Index: ui/views/controls/textfield/textfield.cc
diff --git a/ui/views/controls/textfield/textfield.cc
b/ui/views/controls/textfield/textfield.cc
index
c7e14e23caa8b178177e39ae25e9389654412676..fb1834b3cef99964019d49b18b7314c74023f6b1
100644
--- a/ui/views/controls/textfield/textfield.cc
+++ b/ui/views/controls/textfield/textfield.cc
@@ -7,7 +7,6 @@
 #include <string>
 
 #include "base/debug/trace_event.h"
-#include "base/i18n/case_conversion.h"
 #include "grit/ui_strings.h"
 #include "ui/base/accessibility/accessible_view_state.h"
 #include "ui/base/dragdrop/drag_drop_types.h"
@@ -75,7 +74,6 @@ size_t Textfield::GetCaretBlinkMs() {
 Textfield::Textfield()
     : model_(new TextfieldModel(this)),
       controller_(NULL),
-      style_(STYLE_DEFAULT),
       read_only_(false),
       default_width_in_chars_(0),
       text_color_(SK_ColorBLACK),
@@ -96,7 +94,7 @@ Textfield::Textfield()
   SetFocusable(true);
 
   if (ViewsDelegate::views_delegate) {
-    obscured_reveal_duration_ = ViewsDelegate::views_delegate->
+    password_reveal_duration_ = ViewsDelegate::views_delegate->
         GetDefaultTextfieldObscuredRevealDuration();
   }
 
@@ -104,51 +102,7 @@ Textfield::Textfield()
     focus_painter_ = Painter::CreateDashedFocusPainter();
 }
 
-Textfield::Textfield(StyleFlags style)
-    : model_(new TextfieldModel(this)),
-      controller_(NULL),
-      style_(style),
-      read_only_(false),
-      default_width_in_chars_(0),
-      text_color_(SK_ColorBLACK),
-      use_default_text_color_(true),
-      background_color_(SK_ColorWHITE),
-      use_default_background_color_(true),
-      placeholder_text_color_(kDefaultPlaceholderTextColor),
-      text_input_type_(ui::TEXT_INPUT_TYPE_TEXT),
-      skip_input_method_cancel_composition_(false),
-      is_cursor_visible_(false),
-      is_drop_cursor_visible_(false),
-      initiating_drag_(false),
-      aggregated_clicks_(0),
-      weak_ptr_factory_(this) {
-  set_context_menu_controller(this);
-  set_drag_controller(this);
-  set_border(new FocusableBorder());
-  SetFocusable(true);
-
-  if (IsObscured())
-    SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
-
-  if (ViewsDelegate::views_delegate) {
-    obscured_reveal_duration_ = ViewsDelegate::views_delegate->
-        GetDefaultTextfieldObscuredRevealDuration();
-  }
-
-  if (NativeViewHost::kRenderNativeControlFocus)
-    focus_painter_ = Painter::CreateDashedFocusPainter();
-}
-
-Textfield::~Textfield() {
-}
-
-void Textfield::SetController(TextfieldController* controller) {
-  controller_ = controller;
-}
-
-TextfieldController* Textfield::GetController() const {
-  return controller_;
-}
+Textfield::~Textfield() {}
 
 void Textfield::SetReadOnly(bool read_only) {
   // Update read-only without changing the focusable state (or active, etc.).
@@ -159,34 +113,17 @@ void Textfield::SetReadOnly(bool read_only) {
   UpdateBackgroundColor();
 }
 
-bool Textfield::IsObscured() const {
-  return style_ & STYLE_OBSCURED;
-}
-
-void Textfield::SetObscured(bool obscured) {
-  if (obscured) {
-    style_ = static_cast<StyleFlags>(style_ | STYLE_OBSCURED);
-    SetTextInputType(ui::TEXT_INPUT_TYPE_PASSWORD);
-  } else {
-    style_ = static_cast<StyleFlags>(style_ & ~STYLE_OBSCURED);
-    SetTextInputType(ui::TEXT_INPUT_TYPE_TEXT);
-  }
-  GetRenderText()->SetObscured(obscured);
+void Textfield::SetTextInputType(ui::TextInputType type) {
+  GetRenderText()->SetObscured(type == ui::TEXT_INPUT_TYPE_PASSWORD);
+  text_input_type_ = type;
   OnCaretBoundsChanged();
   if (GetInputMethod())
     GetInputMethod()->OnTextInputTypeChanged(this);
   SchedulePaint();
 }
 
-void Textfield::SetTextInputType(ui::TextInputType type) {
-  text_input_type_ = type;
-  bool should_be_obscured = type == ui::TEXT_INPUT_TYPE_PASSWORD;
-  if (IsObscured() != should_be_obscured)
-    SetObscured(should_be_obscured);
-}
-
 void Textfield::SetText(const base::string16& new_text) {
-  model_->SetText(GetTextForDisplay(new_text));
+  model_->SetText(new_text);
   OnCaretBoundsChanged();
   SchedulePaint();
   NotifyAccessibilityEvent(ui::AccessibilityTypes::EVENT_TEXT_CHANGED, true);
@@ -195,7 +132,7 @@ void Textfield::SetText(const base::string16& new_text) {
 void Textfield::AppendText(const base::string16& new_text) {
   if (new_text.empty())
     return;
-  model_->Append(GetTextForDisplay(new_text));
+  model_->Append(new_text);
   OnCaretBoundsChanged();
   SchedulePaint();
 }
@@ -411,7 +348,7 @@ bool Textfield::OnKeyPressed(const ui::KeyEvent& event) {
 
     gfx::RenderText* render_text = GetRenderText();
     const bool editable = !read_only();
-    const bool readable = !IsObscured();
+    const bool readable = text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD;
     const bool shift = event.IsShiftDown();
     const bool control = event.IsControlDown();
     const bool alt = event.IsAltDown() || event.IsAltGrDown();
@@ -642,7 +579,7 @@ void Textfield::GetAccessibleState(ui::AccessibleViewState*
state) {
   state->name = accessible_name_;
   if (read_only())
     state->state |= ui::AccessibilityTypes::STATE_READONLY;
-  if (IsObscured())
+  if (text_input_type_ == ui::TEXT_INPUT_TYPE_PASSWORD)
     state->state |= ui::AccessibilityTypes::STATE_PROTECTED;
   state->value = text();
 
@@ -841,7 +778,6 @@ int Textfield::OnPerformDrop(const ui::DropTargetEvent&
event) {
       render_text->FindCursorPosition(event.location());
   base::string16 new_text;
   event.data().GetString(&new_text);
-  new_text = GetTextForDisplay(new_text);
 
   // Delete the current selection for a drag and drop within this view.
   const bool move = initiating_drag_ && !event.IsControlDown() &&
@@ -924,11 +860,13 @@ void Textfield::WriteDragDataForView(views::View* sender,
 int Textfield::GetDragOperationsForView(views::View* sender,
                                         const gfx::Point& p) {
   int drag_operations = ui::DragDropTypes::DRAG_COPY;
-  if (!enabled() || IsObscured() || !GetRenderText()->IsPointInSelection(p))
+  if (!enabled() || text_input_type_ == ui::TEXT_INPUT_TYPE_PASSWORD ||
+      !GetRenderText()->IsPointInSelection(p)) {
     drag_operations = ui::DragDropTypes::DRAG_NONE;
-  else if (sender == this && !read_only())
+  } else if (sender == this && !read_only()) {
     drag_operations =
         ui::DragDropTypes::DRAG_MOVE | ui::DragDropTypes::DRAG_COPY;
+  }
   if (controller_)
     controller_->OnGetDragOperationsForTextfield(&drag_operations);
   return drag_operations;
@@ -1008,13 +946,14 @@ bool Textfield::IsCommandIdChecked(int command_id) const
{
 bool Textfield::IsCommandIdEnabled(int command_id) const {
   base::string16 result;
   bool editable = !read_only();
+  bool readable = text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD;
   switch (command_id) {
     case IDS_APP_UNDO:
       return editable && model_->CanUndo();
     case IDS_APP_CUT:
-      return editable && model_->HasSelection() && !IsObscured();
+      return editable && readable && model_->HasSelection();
     case IDS_APP_COPY:
-      return model_->HasSelection() && !IsObscured();
+      return readable && model_->HasSelection();
     case IDS_APP_PASTE:
       ui::Clipboard::GetForCurrentThread()->ReadText(
           ui::CLIPBOARD_TYPE_COPY_PASTE, &result);
@@ -1128,9 +1067,9 @@ void Textfield::InsertText(const base::string16& new_text)
{
   OnBeforeUserAction();
   skip_input_method_cancel_composition_ = true;
   if (GetRenderText()->insert_mode())
-    model_->InsertText(GetTextForDisplay(new_text));
+    model_->InsertText(new_text);
   else
-    model_->ReplaceText(GetTextForDisplay(new_text));
+    model_->ReplaceText(new_text);
   skip_input_method_cancel_composition_ = false;
   UpdateAfterChange(true, true);
   OnAfterUserAction();
@@ -1154,15 +1093,14 @@ void Textfield::InsertChar(base::char16 ch, int flags) {
     model_->ReplaceChar(ch);
   skip_input_method_cancel_composition_ = false;
 
-  model_->SetText(GetTextForDisplay(text()));
-
   UpdateAfterChange(true, true);
   OnAfterUserAction();
 
-  if (IsObscured() && obscured_reveal_duration_ != base::TimeDelta()) {
+  if (text_input_type_ == ui::TEXT_INPUT_TYPE_PASSWORD &&
+      password_reveal_duration_ != base::TimeDelta()) {
     const size_t change_offset = model_->GetCursorPosition();
     DCHECK_GT(change_offset, 0u);
-    RevealObscuredChar(change_offset - 1, obscured_reveal_duration_);
+    RevealPasswordChar(change_offset - 1);
   }
 }
 
@@ -1335,10 +1273,6 @@ gfx::RenderText* Textfield::GetRenderText() const {

////////////////////////////////////////////////////////////////////////////////
 // Textfield, private:
 
-base::string16 Textfield::GetTextForDisplay(const base::string16& raw) {
-  return style_ & Textfield::STYLE_LOWERCASE ? base::i18n::ToLower(raw) : raw;
-}
-
 void Textfield::AccessibilitySetValue(const base::string16& new_value) {
   if (!read_only()) {
     SetText(new_value);
@@ -1446,7 +1380,8 @@ void Textfield::OnAfterUserAction() {
 }
 
 bool Textfield::Cut() {
-  if (!read_only() && !IsObscured() && model_->Cut()) {
+  if (!read_only() && text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD &&
+      model_->Cut()) {
     if (controller_)
       controller_->OnAfterCutOrCopy();
     return true;
@@ -1455,7 +1390,7 @@ bool Textfield::Cut() {
 }
 
 bool Textfield::Copy() {
-  if (!IsObscured() && model_->Copy()) {
+  if (text_input_type_ != ui::TEXT_INPUT_TYPE_PASSWORD && model_->Copy()) {
     …
(message too large)

[commit-bot: I haz the power, 2014-01-16 07:07:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msw@chromium.org/138363004/350001

[commit-bot: I haz the power, 2014-01-16 18:00:52]

Change committed as 245243