Implement "Look Up In Dictionary" context menu item asynchronously.

content/browser/renderer_host/render_widget_host_view_mac.mm

 // 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 "content/browser/renderer_host/render_widget_host_view_mac.h"
 
 #import <objc/runtime.h>
 #include <OpenGL/gl.h>
 #include <QuartzCore/QuartzCore.h>
 
@@ skipping 1208 matching lines @@
     [cocoa_view_ setMarkedRange:range.ToNSRange()];
   }
 
   RenderWidgetHostViewBase::SelectionChanged(text, offset, range);
 }
 
 void RenderWidgetHostViewMac::SelectionBoundsChanged(
     const ViewHostMsg_SelectionBounds_Params& params) {
   if (params.anchor_rect == params.focus_rect)
     caret_rect_ = params.anchor_rect;
+  first_selection_rect_ = params.anchor_rect;
 }
 
 void RenderWidgetHostViewMac::SetShowingContextMenu(bool showing) {
   RenderWidgetHostViewBase::SetShowingContextMenu(showing);
 
   // Create a fake mouse event to inform the render widget that the mouse
   // left or entered.
   NSWindow* window = [cocoa_view_ window];
   // TODO(asvitkine): If the location outside of the event stream doesn't
   // correspond to the current event (due to delayed event processing), then
@@ skipping 200 matching lines @@
 
 bool RenderWidgetHostViewMac::GetCachedFirstRectForCharacterRange(
     NSRange range,
     NSRect* rect,
     NSRange* actual_range) {
   DCHECK(rect);
   // This exists to make IMEs more responsive, see http://crbug.com/115920
   TRACE_EVENT0("browser",
                "RenderWidgetHostViewMac::GetFirstRectForCharacterRange");
 
+  const gfx::Range requested_range(range);
   // If requested range is same as caret location, we can just return it.
-  if (selection_range_.is_empty() && gfx::Range(range) == selection_range_) {
+  if (selection_range_.is_empty() && requested_range == selection_range_) {
     if (actual_range)
       *actual_range = range;
     *rect = NSRectFromCGRect(caret_rect_.ToCGRect());
     return true;
   }
 
+  if (composition_range_.is_empty()) {
+    if (!selection_range_.Contains(requested_range))
+      return false;
+    if (actual_range)
+      *actual_range = selection_range_.ToNSRange();
+    *rect = NSRectFromCGRect(first_selection_rect_.ToCGRect());
+    return true;
+  }
+
   const gfx::Range request_range_in_composition =
-      ConvertCharacterRangeToCompositionRange(gfx::Range(range));
+      ConvertCharacterRangeToCompositionRange(requested_range);
   if (request_range_in_composition == gfx::Range::InvalidRange())
     return false;
 
   // If firstRectForCharacterRange in WebFrame is failed in renderer,
   // ImeCompositionRangeChanged will be sent with empty vector.
   if (composition_bounds_.empty())
     return false;
   DCHECK_EQ(composition_bounds_.size(), composition_range_.length());
 
   gfx::Range ui_actual_range;
@@ skipping 879 matching lines @@
 
 - (void)smartMagnifyWithEvent:(NSEvent*)event {
   const WebGestureEvent& smartMagnifyEvent =
       WebInputEventFactory::gestureEvent(event, self);
   if (renderWidgetHostView_ && renderWidgetHostView_->render_widget_host_) {
     renderWidgetHostView_->render_widget_host_->ForwardGestureEvent(
         smartMagnifyEvent);
   }
 }
 
-// This is invoked only on 10.8 or newer when the user taps a word using
-// three fingers.
-- (void)quickLookWithEvent:(NSEvent*)event {
-  NSPoint point = [self convertPoint:[event locationInWindow] fromView:nil];
+- (void) ShowLookUpDictionary:(NSPoint)point {
   TextInputClientMac::GetInstance()->GetStringAtPoint(
       renderWidgetHostView_->render_widget_host_,
       gfx::Point(point.x, NSHeight([self frame]) - point.y),
       ^(NSAttributedString* string, NSPoint baselinePoint) {
           if (string && [string length] > 0) {
             dispatch_async(dispatch_get_main_queue(), ^{
                 [self showDefinitionForAttributedString:string
                                                 atPoint:baselinePoint];
             });
           }
       }
   );
 }
 
+// This is invoked only on 10.8 or newer when the user taps a word using
+// three fingers.
+- (void)quickLookWithEvent:(NSEvent*)event {

[Alexei Svitkine (slow), 2015/09/01 16:27:03]

Please separate the dictionary logic change into its own CL, that way your
previous CL to change -attributedSubstringForProposedRange can be kept very
close to what you landed before.

[Shu Chen, 2015/09/02 03:35:42]

Done.

+  NSPoint point = [self convertPoint:[event locationInWindow] fromView:nil];
+  [self ShowLookUpDictionary:point];
+}
+
 // This method handles 2 different types of hardware events.
 // (Apple does not distinguish between them).
 //  a. Scrolling the middle wheel of a mouse.
 //  b. Swiping on the track pad.
 //
 // This method is responsible for 2 types of behavior:
 //  a. Scrolling the content of window.
 //  b. Navigating forwards/backwards in history.
 //
 // This is a brief description of the logic:
@@ skipping 502 matching lines @@
   // there. (When this method returns an empty range, the input method doesn't
   // call the setMarkedText method.)
   return hasMarkedText_ ? markedRange_ : NSMakeRange(NSNotFound, 0);
 }
 
 - (NSAttributedString*)attributedSubstringForProposedRange:(NSRange)range
     actualRange:(NSRangePointer)actualRange {
   // TODO(thakis): Pipe |actualRange| through TextInputClientMac machinery.
   if (actualRange)
     *actualRange = range;
-  NSAttributedString* str =
-      TextInputClientMac::GetInstance()->GetAttributedSubstringFromRange(
-          renderWidgetHostView_->render_widget_host_, range);
-  return str;
+
+  const gfx::Range requested_range(range);
+  if (requested_range.is_reversed())
+    return nil;
+
+  gfx::Range expected_range;
+  const base::string16* expected_text;
+
+  if (!renderWidgetHostView_->composition_range().is_empty()) {
+    expected_text = &markedText_;
+    expected_range = renderWidgetHostView_->composition_range();
+  } else {
+    expected_text = &renderWidgetHostView_->selection_text();
+    size_t offset = renderWidgetHostView_->selection_text_offset();
+    expected_range = gfx::Range(offset, offset + expected_text->size());

[palmer, 2015/09/01 18:59:11]

How do you know this range is valid? Can offset + expected_text->size() go
outside the available space?

+  }
+
+  if (!expected_range.Contains(requested_range))
+    return nil;
+
+  // Gets the raw bytes to avoid unnecessary string copies for generating
+  // NSString.
+  const base::char16* bytes =
+      &(*expected_text)[requested_range.start() - expected_range.start()];

[palmer, 2015/09/01 18:59:12]

I don't understand this code. What is going on?

What guarantees that the expression "requested_range.start() -
expected_range.start()" describes a valid range of memory?

+  NSUInteger bytes_len = requested_range.length() * sizeof(base::char16);

[palmer, 2015/09/01 18:59:12]

Is NSUInteger guaranteed to have the same range as a size_t? If not, there could
be integer truncation here.

You might want to make use of base/numerics/safe_conversions.h.

+  base::scoped_nsobject<NSString> ns_string(
+      [[NSString alloc] initWithBytes:bytes
+                               length:bytes_len
+                             encoding:NSUTF16LittleEndianStringEncoding]);
+  return [[[NSAttributedString alloc] initWithString:ns_string] autorelease];
 }
 
 - (NSInteger)conversationIdentifier {
   return reinterpret_cast<NSInteger>(self);
 }
 
 // Each RenderWidgetHostViewCocoa has its own input context, but we return
 // nil when the caret is in non-editable content or password box to avoid
 // making input methods do their work.
 - (NSTextInputContext *)inputContext {
@@ skipping 411 matching lines @@
 
 // "-webkit-app-region: drag | no-drag" is implemented on Mac by excluding
 // regions that are not draggable. (See ControlRegionView in
 // native_app_window_cocoa.mm). This requires the render host view to be
 // draggable by default.
 - (BOOL)mouseDownCanMoveWindow {
   return YES;
 }
 
 @end