Omnibox UI Refresh: Make "invisible focus" apperance more consistent

On the New Tab Page, when the user clicks the fake Google search box,
i.e. the "fakebox", we give the Omnibox "invisible focus".

This is a unique state where we set the Omnibox is focused and
capturing key events, but the caret is invisible.

In the past, the caret being invisible was sufficient for the Omnibox
to be "invisibly" focused.

In MD Refresh, however, we have three new focus indicators:
 1. Focus ring
 2. Placeholder text
 3. Background color change

This CL updates all three of the above visual indicators for the
Omnibox to be keyed on the invisible-focus state rather than the
ordinary focus state.

This solves the flicker present in the below bug and actually follows
the concept of invisible-focus correctly.

The concept of invisible-focus is kind of evil, and long term, we would
like to get rid of it -- but this is a short term fix for MD Refresh.

Bug: 859826
Change-Id: I2e0ede93e69485745bbf119a3f366b0dbfa197ab
Reviewed-on: https://chromium-review.googlesource.com/1144211
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576752}

chrome/browser/ui/views/location_bar/location_bar_view.cc

@@ -785,8 +785,9 @@ void LocationBarView::RefreshBackground() {
   SkColor border_color = GetBorderColor();
 
   if (IsRounded()) {
-    // Match the background color to the popup if the Omnibox is focused.
-    if (omnibox_view_->HasFocus()) {
+    // Match the background color to the popup if the Omnibox is visibly
+    // focused.
+    if (omnibox_view_->model()->is_caret_visible()) {
       background_color = border_color =
           GetColor(OmniboxPart::RESULTS_BACKGROUND);
     }
@@ -916,8 +917,9 @@ void LocationBarView::RefreshFocusRing() {
     focus_ring_->SetHasFocusPredicate([](View* view) -> bool {
       auto* v = static_cast<LocationBarView*>(view);
 
-      // Show focus ring when the Omnibox is focused and the popup is closed.
-      return v->omnibox_view_->HasFocus() &&
+      // Show focus ring when the Omnibox is visibly focused and the popup is
+      // closed.
+      return v->omnibox_view_->model()->is_caret_visible() &&
              !v->GetOmniboxPopupView()->IsOpen();
     });
   }

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -228,7 +228,6 @@ void OmniboxViewViews::ResetTabState(content::WebContents* web_contents) {
 void OmniboxViewViews::InstallPlaceholderText() {
   set_placeholder_text_color(
       location_bar_view_->GetColor(OmniboxPart::LOCATION_BAR_TEXT_DIMMED));
-  set_placeholder_text_hidden_on_focus(true);
 
   const TemplateURL* const default_provider =
       model()->client()->GetTemplateURLService()->GetDefaultSearchProvider();
@@ -547,6 +546,13 @@ void OmniboxViewViews::UpdatePopup() {
 
 void OmniboxViewViews::ApplyCaretVisibility() {
   SetCursorEnabled(model()->is_caret_visible());
+
+  // TODO(tommycli): Because the LocationBarView has a somewhat different look
+  // depending on whether or not the caret is visible, we have to resend a
+  // "focused" notification. Remove this once we get rid of the concept of
+  // "invisible focus".
+  if (location_bar_view_)
+    location_bar_view_->OnOmniboxFocused();
 }
 
 void OmniboxViewViews::OnTemporaryTextMaybeChanged(
@@ -1163,6 +1169,10 @@ void OmniboxViewViews::ExecuteTextEditCommand(ui::TextEditCommand command) {
   }
 }
 
+bool OmniboxViewViews::ShouldShowPlaceholderText() const {
+  return Textfield::ShouldShowPlaceholderText() && !model()->is_caret_visible();
+}
+
 #if defined(OS_CHROMEOS)
 void OmniboxViewViews::CandidateWindowOpened(
       chromeos::input_method::InputMethodManager* manager) {

chrome/browser/ui/views/omnibox/omnibox_view_views.h

@@ -235,6 +235,7 @@ class OmniboxViewViews : public OmniboxView,
   void DoInsertChar(base::char16 ch) override;
   bool IsTextEditCommandEnabled(ui::TextEditCommand command) const override;
   void ExecuteTextEditCommand(ui::TextEditCommand command) override;
+  bool ShouldShowPlaceholderText() const override;
 
   // chromeos::input_method::InputMethodManager::CandidateWindowObserver:
 #if defined(OS_CHROMEOS)

components/omnibox/browser/omnibox_edit_model.h

@@ -230,6 +230,8 @@ class OmniboxEditModel {
 
   OmniboxFocusState focus_state() const { return focus_state_; }
   bool has_focus() const { return focus_state_ != OMNIBOX_FOCUS_NONE; }
+
+  // This is the same as when the Omnibox is visibly focused.
   bool is_caret_visible() const {
     return focus_state_ == OMNIBOX_FOCUS_VISIBLE;
   }

ui/views/controls/textfield/textfield.cc

@@ -1997,6 +1997,10 @@ bool Textfield::IsDropCursorForInsertion() const {
   return true;
 }
 
+bool Textfield::ShouldShowPlaceholderText() const {
+  return text().empty() && !GetPlaceholderText().empty();
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // Textfield, private:
 
@@ -2149,8 +2153,7 @@ void Textfield::PaintTextAndCursor(gfx::Canvas* canvas) {
 
   // Draw placeholder text if needed.
   gfx::RenderText* render_text = GetRenderText();
-  if (text().empty() && !GetPlaceholderText().empty() &&
-      (!placeholder_text_hidden_on_focus_ || !HasFocus())) {
+  if (ShouldShowPlaceholderText()) {
     // Disable subpixel rendering when the background color is not opaque
     // because it draws incorrect colors around the glyphs in that case.
     // See crbug.com/786343

ui/views/controls/textfield/textfield.h

@@ -187,10 +187,6 @@ class VIEWS_EXPORT Textfield : public View,
     placeholder_text_draw_flags_ = flags;
   }
 
-  void set_placeholder_text_hidden_on_focus(bool hidden) {
-    placeholder_text_hidden_on_focus_ = hidden;
-  }
-
   // Sets whether to indicate the textfield has invalid content.
   void SetInvalid(bool invalid);
   bool invalid() const { return invalid_; }
@@ -392,6 +388,10 @@ class VIEWS_EXPORT Textfield : public View,
   // else (like replace the text entirely).
   virtual bool IsDropCursorForInsertion() const;
 
+  // Returns true if the placeholder text should be shown. Subclasses may
+  // override this to customize when the placeholder text is shown.
+  virtual bool ShouldShowPlaceholderText() const;
+
  private:
   friend class TextfieldTestApi;
 
@@ -554,10 +554,6 @@ class VIEWS_EXPORT Textfield : public View,
   // placeholder text uses the same font list as the underlying RenderText.
   base::Optional<gfx::FontList> placeholder_font_list_;
 
-  // If this is true, the placeholder text is never drawn when the Textfield is
-  // focused, regardless of whether or not it's empty.
-  bool placeholder_text_hidden_on_focus_ = false;
-
   // True when the contents are deemed unacceptable and should be indicated as
   // such.
   bool invalid_;