Revert "[Mac] Add Offset to TextSuggestionsTouchBarController Text Selection"

This reverts commit 1e3c1743.

Reason for revert: Breaks Mac tests, see e.g. https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8938549477492727648/+/steps/browser_tests_on__none__GPU_on_Mac_on_Mac-10.11/0/logs/TextSuggestionsTouchBarControllerTest.Offset/0

Original change's description:
> [Mac] Add Offset to TextSuggestionsTouchBarController Text Selection
> 
> When the text surrounding the current selection gets too long, only
> part of it is received by TextSuggestionsTouchBarController. The
> location of the received text within the total block of text is
> denoted by an offset. Previously, TextSuggestionsTouchBarController
> did not take this offset into account, causing crashes when the
> selection range was past the end of the received text.
> 
> The offset is passed along with the text and selection range. The
> selection range within the received text is calculated using the
> offset and received selection range. The editing word range is stored
> both as the range within the received text and the range within the
> total text for checking when to ignore text selection updates.
> 
> RenderWidgetHostView::GetOffsetForSurroundingText() was added to
> retrieve the offset on WebContents update.
> 
> Browser tests were modified to account for the new offset parameter. A
> new browser test, TextSuggestionsTouchBarController.Offset, was added
> to test that the controller properly handles offsets.
> 
> Bug: 717553
> Change-Id: I6dce1405fd5c115ca6ddfdbe4157faf75ccc3a6c
> Reviewed-on: https://chromium-review.googlesource.com/1169597
> Commit-Queue: Tessa Nijssen <tnijssen@google.com>
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Sarah Chan <spqchan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#582205}

TBR=avi@chromium.org,spqchan@chromium.org,tnijssen@google.com

Change-Id: If0acdd0494b5270c4b4e221e16b8e901ce2cacc3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 717553
Reviewed-on: https://chromium-review.googlesource.com/1171704Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582423}

chrome/browser/ui/cocoa/touchbar/text_suggestions_touch_bar_controller.h

@@ -45,8 +45,7 @@ class Range;
     API_AVAILABLE(macos(10.12.2));
 
 - (void)updateTextSelection:(const base::string16&)text
-                      range:(const gfx::Range&)range
-                     offset:(size_t)offset;
+                      range:(const gfx::Range&)range;
 
 // Returns a range from start to the end of the word that the cursor is
 // currently in.
@@ -73,7 +72,7 @@ class Range;
 - (NSArray*)suggestions;
 - (WebTextfieldTouchBarController*)controller;
 - (void)setShouldIgnoreReplacementSelection:(BOOL)shouldIgnore;
-- (void)setEditingWordRange:(const gfx::Range&)range offset:(size_t)offset;
+- (void)setEditingWordRange:(const gfx::Range&)range;
 
 @end
 

chrome/browser/ui/cocoa/touchbar/text_suggestions_touch_bar_controller.mm

@@ -32,14 +32,13 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   }
 
   void DidChangeTextSelection(const base::string16& text,
-                              const gfx::Range& range,
-                              size_t offset) override {
-    [owner_ updateTextSelection:text range:range offset:offset];
+                              const gfx::Range& range) override {
+    [owner_ updateTextSelection:text range:range];
   }
 
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override {
-    [owner_ updateTextSelection:base::string16() range:gfx::Range() offset:0];
+    [owner_ updateTextSelection:base::string16() range:gfx::Range()];
   }
 
  private:
@@ -74,11 +73,6 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   // cursor.
   NSRange editingWordRange_;
 
-  // The location of |editingWordRange_| within the total text, which may be
-  // longer than the text received on text selection update. Used for checking
-  // when to ignore replacement text selections.
-  NSRange offsetEditingWordRange_;
-
   // When YES, -updateTextSelection:range: should ignore a text selection that
   // is equal to the editing word range. Set to YES when
   // -replaceEditingWordWithSuggestion: modifies the current text selection.
@@ -159,11 +153,10 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 }
 
 - (void)updateTextSelection:(const base::string16&)text
-                      range:(const gfx::Range&)range
-                     offset:(size_t)offset {
+                      range:(const gfx::Range&)range {
   if (@available(macOS 10.12.2, *)) {
     if (shouldIgnoreReplacementSelection_ &&
-        range == gfx::Range(offsetEditingWordRange_)) {
+        range == gfx::Range(editingWordRange_)) {
       shouldIgnoreReplacementSelection_ = NO;
       return;
     }
@@ -174,13 +167,9 @@ class WebContentsTextObserver : public content::WebContentsObserver {
     }
 
     text_.reset([base::SysUTF16ToNSString(text) retain]);
-    selectionRange_ =
-        NSMakeRange(range.start() - offset, range.end() - range.start());
+    selectionRange_ = range.ToNSRange();
     editingWordRange_ =
-        [self editingWordRangeFromText:text
-                        cursorPosition:selectionRange_.location];
-    offsetEditingWordRange_ = NSMakeRange(editingWordRange_.location + offset,
-                                          editingWordRange_.length);
+        [self editingWordRangeFromText:text cursorPosition:range.start()];
     [self requestSuggestions];
   }
 }
@@ -266,12 +255,10 @@ class WebContentsTextObserver : public content::WebContentsObserver {
       webContents_->GetTopLevelRenderWidgetHostView()->GetSurroundingText();
   const gfx::Range range =
       webContents_->GetTopLevelRenderWidgetHostView()->GetSelectedRange();
-  const size_t offset = webContents_->GetTopLevelRenderWidgetHostView()
-                            ->GetOffsetForSurroundingText();
   if (range.IsValid())
-    [self updateTextSelection:text range:range offset:offset];
+    [self updateTextSelection:text range:range];
   else
-    [self updateTextSelection:base::string16() range:gfx::Range() offset:0];
+    [self updateTextSelection:base::string16() range:gfx::Range()];
 }
 
 - (content::WebContents*)webContents {
@@ -310,10 +297,8 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   shouldIgnoreReplacementSelection_ = shouldIgnore;
 }
 
-- (void)setEditingWordRange:(const gfx::Range&)range offset:(size_t)offset {
+- (void)setEditingWordRange:(const gfx::Range&)range {
   editingWordRange_ = range.ToNSRange();
-  offsetEditingWordRange_ = NSMakeRange(editingWordRange_.location + offset,
-                                        editingWordRange_.length);
 }
 
 @end

chrome/browser/ui/cocoa/touchbar/text_suggestions_touch_bar_controller_browsertest.mm

@@ -7,7 +7,6 @@
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/strings/sys_string_conversions.h"
-#include "base/strings/utf_string_conversions.h"
 #include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/ui/browser.h"
@@ -147,8 +146,7 @@ IN_PROC_BROWSER_TEST_F(TextSuggestionsTouchBarControllerTest,
   [web_textfield_controller_ resetNumInvalidations];
 
   [touch_bar_controller_ updateTextSelection:base::SysNSStringToUTF16(kText)
-                                       range:kRange
-                                      offset:0];
+                                       range:kRange];
   EXPECT_STREQ(kEmptyText.UTF8String, [touch_bar_controller_ text].UTF8String);
   EXPECT_EQ(kEmptyRange, [touch_bar_controller_ selectionRange]);
   if (@available(macOS 10.12.2, *))
@@ -167,8 +165,7 @@ IN_PROC_BROWSER_TEST_F(TextSuggestionsTouchBarControllerTest,
   [web_textfield_controller_ resetNumInvalidations];
 
   [touch_bar_controller_ updateTextSelection:base::SysNSStringToUTF16(kText)
-                                       range:kRange
-                                      offset:0];
+                                       range:kRange];
   if (@available(macOS 10.12.2, *)) {
     EXPECT_STREQ(kText.UTF8String, [touch_bar_controller_ text].UTF8String);
     EXPECT_EQ(kRange, [touch_bar_controller_ selectionRange]);
@@ -226,20 +223,16 @@ IN_PROC_BROWSER_TEST_F(TextSuggestionsTouchBarControllerTest,
   // If ignoreReplacementSelection is YES and new selection range is equal to
   // editing word range, ignore text selection update.
   [touch_bar_controller_ setShouldIgnoreReplacementSelection:YES];
-  [touch_bar_controller_ setEditingWordRange:kEmptyRange offset:0];
-  [touch_bar_controller_ updateTextSelection:kEmptyText
-                                       range:kEmptyRange
-                                      offset:0];
+  [touch_bar_controller_ setEditingWordRange:kEmptyRange];
+  [touch_bar_controller_ updateTextSelection:kEmptyText range:kEmptyRange];
   EXPECT_STREQ(kText.UTF8String, [touch_bar_controller_ text].UTF8String);
   EXPECT_EQ(kRange, [touch_bar_controller_ selectionRange]);
 
   // If ignoreReplacementSelection is YES but new selection range is not equal
   // to editing word range, do not ignore text selection update.
   [touch_bar_controller_ setShouldIgnoreReplacementSelection:YES];
-  [touch_bar_controller_ setEditingWordRange:kRange offset:0];
-  [touch_bar_controller_ updateTextSelection:kEmptyText
-                                       range:kEmptyRange
-                                      offset:0];
+  [touch_bar_controller_ setEditingWordRange:kRange];
+  [touch_bar_controller_ updateTextSelection:kEmptyText range:kEmptyRange];
   if (@available(macOS 10.12.2, *)) {
     EXPECT_STREQ("", [touch_bar_controller_ text].UTF8String);
     EXPECT_EQ(gfx::Range(), [touch_bar_controller_ selectionRange]);
@@ -254,10 +247,8 @@ IN_PROC_BROWSER_TEST_F(TextSuggestionsTouchBarControllerTest,
   // If ignoreReplacementSelection is NO and new selection range is equal to
   // editing word range, do not ignore text selection update.
   [touch_bar_controller_ setShouldIgnoreReplacementSelection:NO];
-  [touch_bar_controller_ setEditingWordRange:kEmptyRange offset:0];
-  [touch_bar_controller_ updateTextSelection:kEmptyText
-                                       range:kEmptyRange
-                                      offset:0];
+  [touch_bar_controller_ setEditingWordRange:kEmptyRange];
+  [touch_bar_controller_ updateTextSelection:kEmptyText range:kEmptyRange];
   if (@available(macOS 10.12.2, *)) {
     EXPECT_STREQ("", [touch_bar_controller_ text].UTF8String);
     EXPECT_EQ(gfx::Range(), [touch_bar_controller_ selectionRange]);
@@ -267,28 +258,4 @@ IN_PROC_BROWSER_TEST_F(TextSuggestionsTouchBarControllerTest,
   }
 }
 
-// Tests that offsets are properly handled.
-IN_PROC_BROWSER_TEST_F(TextSuggestionsTouchBarControllerTest, Offset) {
-  const base::string16 kText =
-      base::string16(base::ASCIIToUTF16("hello world"));
-  const gfx::Range kRange = gfx::Range(3, 3);
-  const size_t kOffset = 1;
-  const gfx::Range kOffsetRange =
-      gfx::Range(kRange.start() - kOffset, kRange.end() - kOffset);
-
-  FocusTextfield();
-
-  // |selectionRange_| should include offset.
-  [touch_bar_controller_ updateTextSelection:kText range:kRange offset:kOffset];
-  EXPECT_EQ(kOffsetRange, [touch_bar_controller_ selectionRange]);
-
-  // The check for ignoring text selection updates should still work with
-  // offsets.
-  [touch_bar_controller_ setShouldIgnoreReplacementSelection:YES];
-  [touch_bar_controller_ updateTextSelection:kText
-                                       range:gfx::Range(1, 7)
-                                      offset:kOffset];
-  EXPECT_EQ(kOffsetRange, [touch_bar_controller_ selectionRange]);
-}
-
 }  // namespace
\ No newline at end of file

content/browser/frame_host/render_widget_host_view_guest.cc

@@ -354,10 +354,6 @@ gfx::Range RenderWidgetHostViewGuest::GetSelectedRange() {
   return platform_view_->GetSelectedRange();
 }
 
-size_t RenderWidgetHostViewGuest::GetOffsetForSurroundingText() {
-  return platform_view_->GetOffsetForSurroundingText();
-}
-
 void RenderWidgetHostViewGuest::SetNeedsBeginFrames(bool needs_begin_frames) {
   if (platform_view_)
     platform_view_->SetNeedsBeginFrames(needs_begin_frames);

content/browser/frame_host/render_widget_host_view_guest.h

@@ -82,7 +82,6 @@ class CONTENT_EXPORT RenderWidgetHostViewGuest
   base::string16 GetSelectedText() override;
   base::string16 GetSurroundingText() override;
   gfx::Range GetSelectedRange() override;
-  size_t GetOffsetForSurroundingText() override;
   void SetNeedsBeginFrames(bool needs_begin_frames) override;
   TouchSelectionControllerClientManager*
   GetTouchSelectionControllerClientManager() override;

content/browser/renderer_host/render_widget_host_delegate.h

@@ -138,8 +138,7 @@ class CONTENT_EXPORT RenderWidgetHostDelegate {
 
 #if defined(OS_MACOSX)
   virtual void DidChangeTextSelection(const base::string16& text,
-                                      const gfx::Range& range,
-                                      size_t offset) {}
+                                      const gfx::Range& range) {}
 #endif
 
   // Request the renderer to Move the caret to the new position.

content/browser/renderer_host/render_widget_host_view_base.cc

@@ -233,12 +233,6 @@ gfx::Range RenderWidgetHostViewBase::GetSelectedRange() {
   return GetTextInputManager()->GetTextSelection(this)->range();
 }
 
-size_t RenderWidgetHostViewBase::GetOffsetForSurroundingText() {
-  if (!GetTextInputManager())
-    return 0;
-  return GetTextInputManager()->GetTextSelection(this)->offset();
-}
-
 void RenderWidgetHostViewBase::SetBackgroundColor(SkColor color) {
   DCHECK(SkColorGetA(color) == SK_AlphaOPAQUE ||
          SkColorGetA(color) == SK_AlphaTRANSPARENT);

content/browser/renderer_host/render_widget_host_view_base.h

@@ -117,7 +117,6 @@ class CONTENT_EXPORT RenderWidgetHostViewBase
   base::string16 GetSelectedText() override;
   base::string16 GetSurroundingText() override;
   gfx::Range GetSelectedRange() override;
-  size_t GetOffsetForSurroundingText() override;
   bool IsMouseLocked() override;
   bool LockKeyboard(base::Optional<base::flat_set<ui::DomCode>> codes) override;
   void SetBackgroundColor(SkColor color) override;

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -592,8 +592,8 @@ void RenderWidgetHostViewMac::OnTextSelectionChanged(
   ns_view_bridge_->SetTextSelection(selection->text(), selection->offset(),
                                     selection->range());
   if (host() && host()->delegate())
-    host()->delegate()->DidChangeTextSelection(
-        selection->text(), selection->range(), selection->offset());
+    host()->delegate()->DidChangeTextSelection(selection->text(),
+                                               selection->range());
 }
 
 bool RenderWidgetHostViewMac::ShouldWaitInPreCommit() {

content/browser/web_contents/web_contents_impl.cc

@@ -3156,10 +3156,9 @@ void WebContentsImpl::SelectRange(const gfx::Point& base,
 
 #if defined(OS_MACOSX)
 void WebContentsImpl::DidChangeTextSelection(const base::string16& text,
-                                             const gfx::Range& range,
-                                             size_t offset) {
+                                             const gfx::Range& range) {
   for (auto& observer : observers_)
-    observer.DidChangeTextSelection(text, range, offset);
+    observer.DidChangeTextSelection(text, range);
 }
 #endif
 

content/browser/web_contents/web_contents_impl.h

@@ -715,8 +715,7 @@ class CONTENT_EXPORT WebContentsImpl : public WebContents,
   void SelectRange(const gfx::Point& base, const gfx::Point& extent) override;
 #if defined(OS_MACOSX)
   void DidChangeTextSelection(const base::string16& text,
-                              const gfx::Range& range,
-                              size_t offset) override;
+                              const gfx::Range& range) override;
 #endif
   void MoveCaret(const gfx::Point& extent) override;
   void AdjustSelectionByCharacterOffset(int start_adjust,

content/public/browser/render_widget_host_view.h

@@ -141,16 +141,12 @@ class CONTENT_EXPORT RenderWidgetHostView {
   // Returns the currently selected text.
   virtual base::string16 GetSelectedText() = 0;
 
-  // Returns part of the text on the page which includes the selected text plus
-  // possibly several characters before and after it.
+  // Returns the currently selected text with the text before and after it.
   virtual base::string16 GetSurroundingText() = 0;
 
   // Returns the range of the selection in the page.
   virtual gfx::Range GetSelectedRange() = 0;
 
-  // The offset of the surrounding text relative to the start of the total text.
-  virtual size_t GetOffsetForSurroundingText() = 0;
-
   // This only returns non-null on platforms that implement touch
   // selection editing (TSE), currently Aura and (soon) Android.
   // TODO(wjmaclean): update this comment when OOPIF TSE is implemented on

content/public/browser/web_contents_observer.h

@@ -449,8 +449,7 @@ class CONTENT_EXPORT WebContentsObserver : public IPC::Listener {
 
   // Invoked when text selection is changed.
   virtual void DidChangeTextSelection(const base::string16& text,
-                                      const gfx::Range& range,
-                                      size_t offset) {}
+                                      const gfx::Range& range) {}
 
   // Invoked when media is playing or paused.  |id| is unique per player and per
   // RenderFrameHost.  There may be multiple players within a RenderFrameHost