Omnibox: Fix unnecessary inside border

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

 // Copyright (c) 2012 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/views/location_bar/location_bar_view.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/command_line.h"
@@ skipping 230 matching lines @@
   const SkColor background_color = GetColor(BACKGROUND);
   location_icon_view_ = new LocationIconView(font_list, this);
   location_icon_view_->set_drag_controller(this);
   AddChildView(location_icon_view_);
 
   // Initialize the Omnibox view.
   omnibox_view_ = new OmniboxViewViews(
       this, profile(), command_updater(), is_popup_mode_, this, font_list);
   omnibox_view_->Init();
   omnibox_view_->SetFocusBehavior(FocusBehavior::ALWAYS);
+  omnibox_view_->SetBorder(views::CreateEmptyBorder(0, 0, 0, 0));

[Marc Treib, 2016/11/25 10:25:41]

Turns out that SetBorder call did something after all :)  (let this be a lesson
to myself to always test stuff before landing, even if "trivial"...)

The SetBorder call used to be in Layout(), now it's in Init(), which should be
fine because the values now never change during the lifetime of the view.

[Peter Kasting, 2016/11/26 06:15:35]

So what does this do?  It's not at all apparent to me why we need to set this. 
("Because it looks wrong if we don't" isn't what I mean; I mean, why are we
getting some other kind of border otherwise?)

Nit: CreateEmptyBorder(gfx::Insets()) does slightly less work.

[Marc Treib, 2016/11/28 10:46:48]

views::Textfield has a FocusableBorder by default:
https://cs.chromium.org/chromium/src/ui/views/controls/textfield/textfield.cc...
Done.

Arguably, this should be done by OmniboxViewViews itself; I see no reason why
its user should have to set it. Same for the SetFocusBehavior call above (though
this seems to be the default anyway...)
WDYT?

[Peter Kasting, 2016/11/28 20:00:13]

Interesting.  So I guess the comment on all this should be something like
"cancel the default FocusableBorder on Textfield, since the LocationBarView will
indicate the focus state."
I just found this that seems even better?:

SetBorder(views::NullBorder());
I agree.  OmniboxViewViews should take care of both of these as necessary, and
the SetFocusBehavior() call isn't even necessary, so I'd just remove that, and
simply move the border setting into OmniboxViewViews().

[Marc Treib, 2016/11/29 14:10:06]

Done.
Done.
Done.

   AddChildView(omnibox_view_);
 
   // Initialize the inline autocomplete view which is visible only when IME is
   // turned on.  Use the same font with the omnibox and highlighted background.
   ime_inline_autocomplete_view_ = new views::Label(base::string16(), font_list);
   ime_inline_autocomplete_view_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
   ime_inline_autocomplete_view_->SetAutoColorReadabilityEnabled(false);
   ime_inline_autocomplete_view_->set_background(
       views::Background::CreateSolidBackground(GetNativeTheme()->GetSystemColor(
           ui::NativeTheme::kColorId_TextfieldSelectionBackgroundFocused)));
@@ skipping 1001 matching lines @@
 // LocationBarView, private TemplateURLServiceObserver implementation:
 
 void LocationBarView::OnTemplateURLServiceChanged() {
   template_url_service_->RemoveObserver(this);
   template_url_service_ = nullptr;
   // If the browser is no longer active, let's not show the info bubble, as this
   // would make the browser the active window again.
   if (omnibox_view_ && omnibox_view_->GetWidget()->IsActive())
     ShowFirstRunBubble();
 }