Mac OS X: Show the Translate icon on Omnibox

chrome/browser/ui/cocoa/translate/translate_bubble_controller.mm

+// Copyright 2014 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.
+
+#import "chrome/browser/ui/cocoa/translate/translate_bubble_controller.h"
+
+#include "base/mac/scoped_nsobject.h"
+#include "chrome/browser/translate/translate_ui_delegate.h"
+#import "chrome/browser/ui/cocoa/browser_window_controller.h"
+#import "chrome/browser/ui/cocoa/info_bubble_view.h"
+#import "chrome/browser/ui/cocoa/info_bubble_window.h"
+#import "chrome/browser/ui/cocoa/toolbar/toolbar_controller.h"
+#include "chrome/browser/ui/translate/translate_bubble_model_impl.h"
+#import "ui/base/cocoa/flipped_view.h"
+
+const CGFloat kWindowWidth = 320;
+
+@implementation TranslateBubbleController
+
+static TranslateBubbleController* translateBubbleController_ = NULL;

[groby-ooo-7-16, 2014/02/11 00:43:55]

Is there any way for an object to hold this controller, as opposed to a global?

E.g. the ZoomBubbleController is stored on the ZoomDecoration.

[hajimehoshi, 2014/02/25 09:31:07]

Hmm, like StarDecoration, I implemented to show the bubble via the command
|command_updater_|. It is because there are multiple paths to show the bubble.
Of course one is clicking the icon and another is, for example, clicking the
context menu to translate the page.

Actually, I've already implemented it in this way for Translate bubble on Aura,
so it'd be quite different from Aura implementation.

Should TranslateDecoration own the TranslateBubble?

[groby-ooo-7-16, 2014/02/26 03:13:58]

I think you should stick with the CommandUpdater solution. I don't think this
should be a global object, though. If you follow what the Bookmark bubble is
doing, bookmarkBubbleController_ is stored on the BrowserWindowController. Maybe
that's a better place?

[hajimehoshi, 2014/02/26 09:18:18]

Thanks. I agree to move this. Done.

+
++ (void)
+    showBubbleWithBrowserWindowController:(BrowserWindowController*)controller

[groby-ooo-7-16, 2014/02/11 00:43:55]

Please keep return type and method name on same line.

In general, I don't think you need to pass in the BWC - "showForParentWindow"
should be enough. (And then -initForParentWindow)

[hajimehoshi, 2014/02/25 09:31:07]

Done.

+                              webContents:(content::WebContents*)webContents
+                                viewState:
+                                    (TranslateBubbleModel::ViewState)viewState
+                                errorType:(TranslateErrors::Type)errorType {
+  if (translateBubbleController_) {
+    // When the user reads the advanced setting panel, the bubble should not be
+    // changed because he/she is focusing on the bubble.
+    if (translateBubbleController_->webContents_ == webContents &&

[groby-ooo-7-16, 2014/02/11 00:43:55]

I'm curious - can translateBubbleController ever have different webContents?
Shouldn't the bubble close whenever you navigate or switch tabs?

[hajimehoshi, 2014/02/25 09:31:07]

No, translatebubbleController can have only one webContents. However, even
though a bubble is shown, showBubbleWithBrowserWindowController can be called
when the automatic translation is enabled. In this case, the current bubble
should be closed. This 'if' clause is true when the user opens a page, shows the
Translate bubble by clicking the icon and switches the advanced menu BEFORE the
automatic translation starts. In this case, the webContents is not changed while
showBubbleWithBrowserWindowController is called twice.

+        translateBubbleController_->model_->GetViewState() ==
+            TranslateBubbleModel::VIEW_STATE_ADVANCED) {
+      return;
+    }
+    if (viewState != TranslateBubbleModel::VIEW_STATE_ERROR)
+      [translateBubbleController_ switchView:viewState];
+    else
+      [translateBubbleController_ switchToErrorView:errorType];
+    return;
+  }
+
+  // fixme(hajimehoshi): Set the initial languages correctly.
+  std::string sourceLanguage = "xx";
+  std::string targetLanguage = "yy";
+
+  scoped_ptr<TranslateUIDelegate> uiDelegate(
+      new TranslateUIDelegate(webContents, sourceLanguage, targetLanguage));
+  scoped_ptr<TranslateBubbleModel> model(
+      new TranslateBubbleModelImpl(viewState, uiDelegate.Pass()));
+  translateBubbleController_ = [[TranslateBubbleController alloc]
+      initWithBrowserWindowController:controller
+                                model:model.Pass()
+                          webContents:webContents];
+  [translateBubbleController_ showWindow:nil];
+}
+
+- (id)initWithBrowserWindowController:(BrowserWindowController*)controller
+                                model:(scoped_ptr<TranslateBubbleModel>)model
+                          webContents:(content::WebContents*)webContents {
+  NSWindow* parentWindow = [controller window];
+
+  // Use an arbitrary size; it will be changed in performLayout.
+  NSRect contentRect = NSMakeRect(0, 0, kWindowWidth, 100);

[groby-ooo-7-16, 2014/02/11 00:43:55]

Use ui::kWindowSizeDeterminedLater, and then set the size in performLayout

[hajimehoshi, 2014/02/25 09:31:07]

Done.

+  base::scoped_nsobject<InfoBubbleWindow> window(
+      [[InfoBubbleWindow alloc] initWithContentRect:contentRect
+                                          styleMask:NSBorderlessWindowMask
+                                            backing:NSBackingStoreBuffered
+                                              defer:NO]);
+
+  if ((self = [super initWithWindow:window
+                       parentWindow:parentWindow
+                         anchoredAt:NSZeroPoint])) {
+    webContents_ = webContents;
+    model_ = model.Pass();
+    translateExecuted_ = NO;
+    if (model_->GetViewState() !=
+        TranslateBubbleModel::VIEW_STATE_BEFORE_TRANSLATE) {
+      translateExecuted_ = YES;
+    }
+
+    // Create the container view that uses flipped coordinates.
+    CGFloat x = info_bubble::kBubbleCornerRadius;
+    CGFloat y = -info_bubble::kBubbleCornerRadius;
+    NSRect contentFrame = NSMakeRect(x, y, kWindowWidth, 100);
+    NSView* contentView = [[FlippedView alloc] initWithFrame:contentFrame];

[groby-ooo-7-16, 2014/02/11 00:43:55]

Unless you need the FlippedView, I would skip this. 

[hajimehoshi, 2014/02/25 09:31:07]

Removed this once. Done.

+    [[window contentView] setSubviews:@[ contentView ]];
+
+    [self performLayout];
+
+    // This class will release itself when the bubble closes. See
+    // -[BaseBubbleController windowWillClose:].
+    [self retain];

[groby-ooo-7-16, 2014/02/11 00:43:55]

The class is already retained through the alloc call. No need to add an extra
retain.

[hajimehoshi, 2014/02/25 09:31:07]

Done.

+  }
+  return self;
+}
+
+- (void)windowWillClose:(NSNotification*)notification {
+  DCHECK_EQ(translateBubbleController_, self);
+  translateBubbleController_ = NULL;
+  [super windowWillClose:notification];
+}
+
+- (void)showWindow:(id)sender {
+  BrowserWindowController* controller = [self.parentWindow windowController];

[groby-ooo-7-16, 2014/02/11 00:43:55]

nit:Since the rest of the file doesn't use dot notation, I'd prefer [self
parentWindow]

[hajimehoshi, 2014/02/25 09:31:07]

Done.

+  NSPoint anchorPoint = [[controller toolbarController] translateBubblePoint];
+  anchorPoint = [self.parentWindow convertBaseToScreen:anchorPoint];
+  [self setAnchorPoint:anchorPoint];
+  [super showWindow:sender];
+}
+
+- (void)switchView:(TranslateBubbleModel::ViewState)viewState {
+  if (model_->GetViewState() == viewState)
+    return;
+
+  model_->SetViewState(viewState);
+  [self performLayout];
+}
+
+- (void)switchToErrorView:(TranslateErrors::Type)errorType {
+  // fixme(hajimehoshi): Implement this.
+}
+
+- (void)performLayout {
+  // fixme(hajimehoshi): Now this shows just an empty bubble. Implement this.
+  [[self window] display];
+}
+
+@end