[Mac] Touch Bar support for default browser window state

chrome/browser/ui/cocoa/browser_window_touch_bar.mm

+// Copyright 2016 The Chromium Authors. All rights reserved.

[Robert Sesek, 2017/02/14 17:32:51]

How about a unit test?

[spqchan, 2017/02/16 15:25:51]

I tried to write some tests, but they're currently flaky. Do you mind if I add
them in a follow up CL? I want to get this landed before the feature freeze

[Robert Sesek, 2017/02/16 18:19:38]

If the test is flaky, doesn't that mean that the underlying code could have a
bug in it?

[spqchan, 2017/02/16 18:35:42]

Sorry, I should've clarified. It's more like I'm trying to get the tests to only
run on > OSX 10.12.1. For some reason, the tests are running on 10.9. All tests
fail on that because NSTouchBar isn't available there, which causes everything
to return nil. 

I'm most likely doing something dumb though so it probably won't take too long
to figure out the issue, but I want to prioritize landing this CL in ASAP before
tomorrow's feature freeze.

[Robert Sesek, 2017/02/16 18:43:22]

Looking at the trybot failures from PS5, this is the error: "+[NSButton
buttonWithImage:target:action:]: unrecognized selector sent to class
0x7fff761eef28". And that's because in -touchBar:makeItemForIdentifier:, while
touchBarItem is nil on 10.9, some of the branches can still call
CreateTouchBarButton(). That method on NSButton is only in the 10.12 SDK.

You could either not use that NSButton method, or just add an early return to
-touchBar:makeItemForIdentifier: if touchBarItem is nil.

Also, you can gate the test body on base::mac::IsAtLeastOS10_12().

[spqchan, 2017/02/16 19:05:23]

Thanks! I was trying to use that a while ago, but there wasn't a method for
IsAtLeastOS10_12_1().
For some reason, MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_12_1
didn't work either
 However, I just realized that I can just get around that by using
!IsAtMostOS10_12() :)

[Robert Sesek, 2017/02/16 19:13:38]

Does the IsAtMostOS10_12() check let the test run on 10.12? I'd probably just
use IsAtLeastOS10_12() and not worry about 10.12.0.

[spqchan, 2017/02/16 19:27:48]

Done.

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#import "chrome/browser/ui/cocoa/browser_window_touch_bar.h"
+
+#include "base/mac/mac_util.h"
+#import "base/mac/scoped_nsobject.h"
+#import "base/mac/sdk_forward_declarations.h"
+#include "base/strings/sys_string_conversions.h"
+#include "chrome/app/chrome_command_ids.h"
+#include "chrome/app/vector_icons/vector_icons.h"
+#include "chrome/browser/command_updater.h"
+#include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_command_controller.h"
+#include "chrome/common/chrome_features.h"
+#include "chrome/grit/generated_resources.h"
+#include "components/search_engines/util.h"
+#include "components/toolbar/vector_icons.h"
+#include "ui/base/l10n/l10n_util.h"
+#include "ui/gfx/color_palette.h"
+#include "ui/gfx/color_utils.h"
+#include "ui/gfx/image/image.h"
+#include "ui/gfx/image/image_skia_util_mac.h"
+#include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/vector_icons_public.h"
+
+namespace {
+
+// The touch bar's identifier.
+const NSTouchBarCustomizationIdentifier kBrowserWindowTouchBarId =
+    @"BrowserWindowTouchBarId";
+
+// Touch bar items identifiers.
+const NSTouchBarItemIdentifier kBackForwardTouchId = @"BackForwardTouchId";
+const NSTouchBarItemIdentifier kReloadOrStopTouchId = @"ReloadOrStopTouchId";
+const NSTouchBarItemIdentifier kSearchTouchId = @"SearchTouchId";
+const NSTouchBarItemIdentifier kNewTabTouchId = @"NewTabTouchId";
+const NSTouchBarItemIdentifier kStarTouchId = @"StarTouchId";
+
+// The button indexes in the back and forward segment control.
+const int kBackSegmentIndex = 0;
+const int kForwardSegmentIndex = 1;
+
+// Touch bar icon colors values.
+const SkColor kTouchBarDefaultIconColor = SK_ColorWHITE;
+const SkColor kTouchBarStarIconColor = gfx::kGoogleBlue500;
+
+// The size of the touch bar icons.
+const int kTouchBarIconSize = 16;
+
+// The width of the search button in the touch bar.
+const int kTouchBarSearchButtonWidth = 280;
+
+// Creates an NSImage from the given VectorIcon.
+NSImage* CreateNSImageFromId(const gfx::VectorIcon* icon,
+                             SkColor color = kTouchBarDefaultIconColor) {
+  return NSImageFromImageSkiaWithColorSpace(
+      gfx::CreateVectorIcon(*icon, kTouchBarIconSize, color),
+      base::mac::GetSRGBColorSpace());
+}
+
+// Creates a NSButton for the touch bar.
+NSButton* CreateTouchBarButton(const gfx::VectorIcon* icon,
+                               BrowserWindowTouchBar* owner,
+                               int command,
+                               SkColor color = kTouchBarDefaultIconColor) {
+  NSButton* button = [NSButton buttonWithImage:CreateNSImageFromId(icon, color)
+                                        target:owner
+                                        action:@selector(executeCommand:)];
+  button.tag = command;
+  return button;
+}
+
+}  // end namespace

[Robert Sesek, 2017/02/14 17:32:51]

Just "namespace"

[spqchan, 2017/02/16 15:25:52]

Done.

+
+@interface BrowserWindowTouchBar () {
+  // Used to execute commands such as navigating back and forward.
+  CommandUpdater* commandUpdater_;  // Weak, owned by Browser.
+
+  // The browser associated with the touch bar.
+  Browser* browser_;  // Weak.
+}
+
+// Creates and return the back and forward segmented buttons.
+- (NSView*)backOrForwardTouchBarView;
+
+// Creates and returns the search button.
+- (NSView*)searchTouchBarView;
+@end
+
+@implementation BrowserWindowTouchBar
+
+@synthesize isPageLoading = isPageLoading_;
+@synthesize isStarred = isStarred_;
+
+- (instancetype)initWithBrowser:(Browser*)browser {
+  if ((self = [self init])) {
+    DCHECK(browser);
+    commandUpdater_ = browser->command_controller()->command_updater();
+    browser_ = browser;
+  }
+
+  return self;
+}
+
+- (NSTouchBar*)makeTouchBar {
+  if (!base::FeatureList::IsEnabled(features::kBrowserTouchBar))
+    return nil;
+
+  base::scoped_nsobject<NSTouchBar> touchBar(
+      [[NSClassFromString(@"NSTouchBar") alloc] init]);
+  NSArray* touchBarItemIdentifiers = @[
+    kBackForwardTouchId, kReloadOrStopTouchId, kSearchTouchId, kNewTabTouchId,
+    kStarTouchId
+  ];
+  [touchBar setCustomizationIdentifier:kBrowserWindowTouchBarId];
+  [touchBar setDefaultItemIdentifiers:touchBarItemIdentifiers];
+  [touchBar setCustomizationAllowedItemIdentifiers:touchBarItemIdentifiers];
+  [touchBar setDelegate:self];
+
+  return touchBar.autorelease();
+}
+
+- (NSTouchBarItem*)touchBar:(NSTouchBar*)touchBar
+      makeItemForIdentifier:(NSTouchBarItemIdentifier)identifier {
+  base::scoped_nsobject<NSCustomTouchBarItem> touchBarItem([[NSClassFromString(
+      @"NSCustomTouchBarItem") alloc] initWithIdentifier:identifier]);
+  if ([identifier isEqualTo:kBackForwardTouchId]) {
+    [touchBarItem setView:[self backOrForwardTouchBarView]];
+  } else if ([identifier isEqualTo:kReloadOrStopTouchId]) {
+    const gfx::VectorIcon* icon =
+        isPageLoading_ ? &kNavigateStopIcon : &kNavigateReloadIcon;
+    int command_id = isPageLoading_ ? IDC_STOP : IDC_RELOAD;
+    [touchBarItem setView:CreateTouchBarButton(icon, self, command_id)];
+  } else if ([identifier isEqualTo:kNewTabTouchId]) {
+    [touchBarItem
+        setView:CreateTouchBarButton(&kNewTabIcon, self, IDC_NEW_TAB)];
+  } else if ([identifier isEqualTo:kStarTouchId]) {
+    const gfx::VectorIcon* icon =
+        isStarred_ ? &toolbar::kStarActiveIcon : &toolbar::kStarIcon;
+    SkColor iconColor =
+        isStarred_ ? kTouchBarStarIconColor : kTouchBarDefaultIconColor;
+    [touchBarItem
+        setView:CreateTouchBarButton(icon, self, IDC_BOOKMARK_PAGE, iconColor)];
+  } else if ([identifier isEqualTo:kSearchTouchId]) {
+    [touchBarItem setView:[self searchTouchBarView]];
+  }
+
+  return touchBarItem.autorelease();
+}
+
+- (NSView*)backOrForwardTouchBarView {
+  NSArray* images = @[
+    CreateNSImageFromId(&kNavigateBackIcon),
+    CreateNSImageFromId(&kNavigateForwardIcon)
+  ];
+
+  NSSegmentedControl* control = [NSSegmentedControl
+      segmentedControlWithImages:images
+                    trackingMode:NSSegmentSwitchTrackingMomentary
+                          target:self
+                          action:@selector(backOrForward:)];
+  control.segmentStyle = NSSegmentStyleSeparated;
+  [control setEnabled:commandUpdater_->IsCommandEnabled(IDC_BACK)
+           forSegment:kBackSegmentIndex];
+  [control setEnabled:commandUpdater_->IsCommandEnabled(IDC_FORWARD)
+           forSegment:kForwardSegmentIndex];
+  return control;
+}
+
+- (NSView*)searchTouchBarView {
+  TemplateURLService* template_url_service =

[Robert Sesek, 2017/02/14 17:32:51]

naming: templateUrlService

[spqchan, 2017/02/16 15:25:52]

Done.

+      TemplateURLServiceFactory::GetForProfile(browser_->profile());
+  const TemplateURL* default_provider =
+      template_url_service->GetDefaultSearchProvider();
+  BOOL isGoogle =
+      default_provider->GetEngineType(
+          template_url_service->search_terms_data()) == SEARCH_ENGINE_GOOGLE;
+
+  base::string16 title = l10n_util::GetStringUTF16(IDS_TOUCH_BAR_SEARCH);
+
+  gfx::VectorIconId iconId = isGoogle ? gfx::VectorIconId::G_SEARCH
+                                      : gfx::VectorIconId::OMNIBOX_SEARCH;
+  NSImage* image = NSImageFromImageSkiaWithColorSpace(
+      gfx::CreateVectorIcon(iconId, kTouchBarIconSize,

[Robert Sesek, 2017/02/14 17:32:51]

Should this use CreateNSImageFromId?

[spqchan, 2017/02/16 15:25:52]

No, iconId is type VectorIconId, which is different from VectorIcon

+                            kTouchBarDefaultIconColor),
+      base::mac::GetSRGBColorSpace());
+
+  NSButton* searchButton =
+      [NSButton buttonWithTitle:base::SysUTF16ToNSString(title)
+                          image:image
+                         target:self
+                         action:@selector(executeCommand:)];
+  searchButton.imageHugsTitle = YES;
+  searchButton.tag = IDC_FOCUS_LOCATION;
+  [searchButton.widthAnchor
+      constraintEqualToConstant:kTouchBarSearchButtonWidth]
+      .active = YES;
+  return searchButton;
+}
+
+- (void)backOrForward:(id)sender {
+  NSSegmentedControl* control = sender;
+  if ([control selectedSegment] == kBackSegmentIndex)
+    commandUpdater_->ExecuteCommand(IDC_BACK);
+  else
+    commandUpdater_->ExecuteCommand(IDC_FORWARD);
+}
+
+- (void)executeCommand:(id)sender {
+  NSButton* button = (NSButton*)sender;

[Robert Sesek, 2017/02/14 17:32:51]

No C-style casts. Use ObjCCast or just do [button tag] instead.

[spqchan, 2017/02/16 15:25:52]

Done.

+  commandUpdater_->ExecuteCommand(button.tag);
+}
+
+@end