Make some updates to BM bar for MD.

chrome/browser/ui/views/bookmarks/bookmark_bar_instructions_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/bookmarks/bookmark_bar_instructions_view.h"
 
 #include <algorithm>
 
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/themes/theme_properties.h"
 #include "chrome/browser/ui/bookmarks/bookmark_bar_instructions_delegate.h"
 #include "chrome/grit/generated_resources.h"
 #include "ui/accessibility/ax_view_state.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/material_design/material_design_controller.h"
 #include "ui/base/theme_provider.h"
+#include "ui/native_theme/native_theme.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/controls/link.h"
 
 namespace {
 
 // Horizontal padding, in pixels, between the link and label.
 const int kViewPadding = 6;
 
 }  // namespace
 
 BookmarkBarInstructionsView::BookmarkBarInstructionsView(
     BookmarkBarInstructionsDelegate* delegate)
     : delegate_(delegate),
       instructions_(NULL),
       import_link_(NULL),
       baseline_(-1),
       updated_colors_(false) {
-  instructions_ = new views::Label(
-      l10n_util::GetStringUTF16(IDS_BOOKMARKS_NO_ITEMS));
+  base::string16 instructions_text =
+      l10n_util::GetStringUTF16(IDS_BOOKMARKS_NO_ITEMS);
+  if (ui::MaterialDesignController::IsModeMaterial())
+    instructions_text += base::ASCIIToUTF16(" ");

[sky, 2015/10/19 15:38:40]

Why do you add a space for material? Might the space have undesirable effects in
other languages?

[Evan Stade, 2015/10/19 16:49:52]

Previously this space was implemented with kViewPadding, but that didn't create
the right amount of space (especially if you change the font). I tried to think
of how this could fail in other languages. It works in RTL and LTR. In languages
without space characters it creates too much space (I just tested with zh_CN),
but that was already a problem. Also it doesn't look that bad because the zh_CN
period character [。] already has some trailing empty space attached, so we get a
slightly too-wide space rather than a space where there should be none.

[jungshik at Google, 2015/10/23 06:30:19]

Sorry for the late reply and thanks for testing in various locales. 
Even though there's no side effect, I really wish we could do without adding an
extra space, but apparently not. 

BTW, "。" (Ideographic full stop) is so-called 'full-width' and its advance width
is the same as other CJK Ideographs. 

+  instructions_ = new views::Label(instructions_text);
   instructions_->SetAutoColorReadabilityEnabled(false);
   AddChildView(instructions_);
 
   if (browser_defaults::kShowImportOnBookmarkBar) {
     import_link_ = new views::Link(
         l10n_util::GetStringUTF16(IDS_BOOKMARK_BAR_IMPORT_LINK));
     // We don't want the link to alter tab navigation.
     import_link_->SetFocusable(false);
     import_link_->set_listener(this);
     import_link_->set_context_menu_controller(this);
     import_link_->SetAutoColorReadabilityEnabled(false);
     AddChildView(import_link_);
   }
 }
 
 gfx::Size BookmarkBarInstructionsView::GetPreferredSize() const {
   int ascent = 0, descent = 0, height = 0, width = 0;
   for (int i = 0; i < child_count(); ++i) {
     const views::View* view = child_at(i);
     gfx::Size pref = view->GetPreferredSize();
     int baseline = view->GetBaseline();
     if (baseline != -1) {
       ascent = std::max(ascent, baseline);
       descent = std::max(descent, pref.height() - baseline);
     } else {
       height = std::max(pref.height(), height);
     }
     width += pref.width();
   }
-  width += (child_count() - 1) * kViewPadding;
+  if (!ui::MaterialDesignController::IsModeMaterial())
+    width += (child_count() - 1) * kViewPadding;
   if (ascent != 0)
     height = std::max(ascent + descent, height);
   return gfx::Size(width, height);
 }
 
 void BookmarkBarInstructionsView::Layout() {
   int remaining_width = width();
   int x = 0;
   for (int i = 0; i < child_count(); ++i) {
     views::View* view = child_at(i);
     gfx::Size pref = view->GetPreferredSize();
     int baseline = view->GetBaseline();
     int y;
     if (baseline != -1 && baseline_ != -1)
       y = baseline_ - baseline;
     else
       y = (height() - pref.height()) / 2;
     int view_width = std::min(remaining_width, pref.width());
     view->SetBounds(x, y, view_width, pref.height());
-    x += view_width + kViewPadding;
+    x += view_width;
+    if (!ui::MaterialDesignController::IsModeMaterial())
+      x += kViewPadding;
     remaining_width = std::max(0, width() - x);
   }
 }
 
 const char* BookmarkBarInstructionsView::GetClassName() const {
   return "BookmarkBarInstructionsView";
 }
 
 void BookmarkBarInstructionsView::OnThemeChanged() {
   UpdateColors();
@@ skipping 25 matching lines @@
 
 void BookmarkBarInstructionsView::UpdateColors() {
   // We don't always have a theme provider (ui tests, for example).
   const ui::ThemeProvider* theme_provider = GetThemeProvider();
   if (!theme_provider)
     return;
   updated_colors_ = true;
   SkColor text_color =
       theme_provider->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT);
   instructions_->SetEnabledColor(text_color);
-  if (import_link_)
-    import_link_->SetEnabledColor(text_color);
+  if (import_link_) {

[varkha, 2015/10/19 15:01:29]

nit: could return early on !import_link to simplify the rest.

[Evan Stade, 2015/10/19 16:49:52]

Done.

+    if (!ui::MaterialDesignController::IsModeMaterial()) {
+      import_link_->SetEnabledColor(text_color);
+    } else {
+      // For MD, use the default link color if it provides enough contrast. If
+      // contrast is too low, fall back to the bookmark text color and use an
+      // underline to make it obvious it's a link. The default color readability
+      // code (which only adjusts luminance) doesn't work well in this case.
+      SkColor bg = theme_provider->GetColor(
+          ThemeProperties::ThemeProperties::COLOR_TOOLBAR);
+      SkColor link_color = GetNativeTheme()->GetSystemColor(
+          ui::NativeTheme::kColorId_LinkEnabled);
+      if (color_utils::GetContrastRatio(link_color, bg) >=
+          color_utils::kMinimumReadableContrastRatio) {
+        import_link_->SetUnderline(false);
+        import_link_->SetEnabledColor(link_color);
+      } else {
+        import_link_->SetUnderline(true);
+        import_link_->SetEnabledColor(text_color);
+      }

[varkha, 2015/10/19 15:01:29]

nit: maybe shorten this similar to:

bool enough_contrast = color_utils::GetContrastRatio(link_color, bg) >=
color_utils::kMinimumReadableContrastRatio;
import_link_->SetUnderline(!enough_contrast);
import_link_->SetEnabledColor(enough_contrast ? link_color : text_color);

[Evan Stade, 2015/10/19 16:49:52]

Done.

+    }
+  }
 }