Revert "[Mac] Re-Generate Text Suggestions on WebContents Update"

This reverts commit 3d248fc5.

Reason for revert: SuggestedTextTouchBarControllerBrowserTest constantly failing after this.

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/28381
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/28382
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/14729
https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/14730

Original change's description:
> [Mac] Re-Generate Text Suggestions on WebContents Update
> 
> Previously, each tab had its own SuggestedTextTouchBarController. Now,
> because of http://crrev/c/1151613, there is only one
> SuggestedTextTouchBarController per window. Because the controller does
> not maintain the state of each tab, as a user switches tabs, the
> controller needs to update its state to match the tab. When the
> controller's WebContents change, new suggestions should be generated
> for the selected text in the current tab.
> 
> The methods -webContentsTextSelectionChanged:range: and
> -webContentsFinishedLoading were combined into a singular method,
> -updateTextSelection:range:, because they had very similar
> functionality.
> 
> The parameters for -requestSuggestionsForText:inRange: were removed
> because they were always |text_| and |selectionRange_|.
> 
> A new method was created, -webContentsChanged, to get a tab's current
> text and selected range and then call -updateTextSelection:range: with
> the proper parameters.
> 
> When the controller gets a new pointer to a WebContents, the pointer
> may be null. To avoid calling WebContents::IsFocusedElementEditable()
> on a null pointer, a method -isTextfieldFocused was created to check
> that |webContents_| is not a null pointer and that an editable element
> is focused.
> 
> A new SuggestedTextTouchBarControllerBrowserTest was written,
> SetWebContentsTest, to test that the SuggestedTextTouchBarController
> properly updates its current text selection based on the new
> WebContents.
> 
> Bug: 717553
> Change-Id: I356a548bbdf33b51484fd7c55a1aa5ea37be4bae
> Reviewed-on: https://chromium-review.googlesource.com/1153728
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Sarah Chan <spqchan@chromium.org>
> Commit-Queue: Tessa Nijssen <tnijssen@google.com>
> Cr-Commit-Position: refs/heads/master@{#579851}

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

Change-Id: I043f5352f43f9ee5eaeff6c7f5a0a9c64b067d26
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 717553
Reviewed-on: https://chromium-review.googlesource.com/1159902Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580052}

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

@@ -19,10 +19,6 @@ namespace content {
 class WebContents;
 }  // namespace content
 
-namespace gfx {
-class Range;
-}  // namespace gfx
-
 @interface SuggestedTextTouchBarController
     : NSObject<NSTouchBarDelegate, NSCandidateListTouchBarItemDelegate>
 
@@ -44,15 +40,19 @@ class Range;
      endSelectingCandidateAtIndex:(NSInteger)index
     API_AVAILABLE(macos(10.12.2));
 
-- (void)updateTextSelection:(const base::string16&)text
-                      range:(const gfx::Range&)range;
+- (void)webContentsTextSelectionChanged:(const base::string16&)text
+                                  range:(NSRange)range
+    API_AVAILABLE(macos(10.12.2));
+
+- (void)webContentsFinishedLoading API_AVAILABLE(macos(10.12.2));
 
 // Returns a range from start to the end of the word that the cursor is
 // currently in.
 - (NSRange)editingWordRangeFromText:(const base::string16&)text
                      cursorPosition:(size_t)cursor;
 
-- (void)requestSuggestions API_AVAILABLE(macos(10.12.2));
+- (void)requestSuggestionsForText:(NSString*)text
+                          inRange:(NSRange)range API_AVAILABLE(macos(10.12.2));
 
 // Select the range of the editing word and replace it with a suggestion
 // from the touch bar.
@@ -69,8 +69,8 @@ class Range;
 - (WebTextfieldTouchBarController*)controller;
 - (void)setWebContents:(content::WebContents*)webContents;
 - (content::WebContents*)webContents;
-- (void)setSelectionRange:(const gfx::Range&)range;
-- (gfx::Range)selectionRange;
+- (void)setSelectionRange:(NSRange)range;
+- (NSRange)selectionRange;
 
 @end
 

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

@@ -6,7 +6,6 @@
 
 #include "base/i18n/break_iterator.h"
 #include "base/strings/sys_string_conversions.h"
-#include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #import "ui/base/cocoa/touch_bar_util.h"
@@ -33,12 +32,16 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 
   void DidChangeTextSelection(const base::string16& text,
                               const gfx::Range& range) override {
-    [owner_ updateTextSelection:text range:range];
+    if (@available(macOS 10.12.2, *)) {
+      [owner_ webContentsTextSelectionChanged:text range:range.ToNSRange()];
+    }
   }
 
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override {
-    [owner_ updateTextSelection:base::string16() range:gfx::Range()];
+    if (@available(macOS 10.12.2, *)) {
+      [owner_ webContentsFinishedLoading];
+    }
   }
 
  private:
@@ -77,10 +80,6 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 
 @implementation SuggestedTextTouchBarController
 
-- (BOOL)isTextfieldFocused {
-  return webContents_ && webContents_->IsFocusedElementEditable();
-}
-
 - (instancetype)initWithWebContents:(content::WebContents*)webContents
                          controller:
                              (WebTextfieldTouchBarController*)controller {
@@ -95,7 +94,7 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 }
 
 - (NSTouchBar*)makeTouchBar {
-  if (![self isTextfieldFocused])
+  if (!webContents_ || !webContents_->IsFocusedElementEditable())
     return nil;
 
   base::scoped_nsobject<NSTouchBar> touchBar([[ui::NSTouchBar() alloc] init]);
@@ -144,19 +143,26 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   ui::LogTouchBarUMA(ui::TouchBarAction::TEXT_SUGGESTION);
 }
 
-- (void)updateTextSelection:(const base::string16&)text
-                      range:(const gfx::Range&)range {
-  if (@available(macOS 10.12.2, *)) {
-    if (![self isTextfieldFocused]) {
+- (void)webContentsTextSelectionChanged:(const base::string16&)text
+                                  range:(NSRange)range {
+  if (webContents_->IsFocusedElementEditable()) {
+    [self setText:base::SysUTF16ToNSString(text)];
+    selectionRange_ = range;
+    editingWordRange_ =
+        [self editingWordRangeFromText:text cursorPosition:range.location];
+    [self requestSuggestionsForText:text_ inRange:range];
+  } else {
     [controller_ invalidateTouchBar];
-      return;
   }
+}
 
-    text_.reset([base::SysUTF16ToNSString(text) retain]);
-    selectionRange_ = range.ToNSRange();
-    editingWordRange_ =
-        [self editingWordRangeFromText:text cursorPosition:range.start()];
-    [self requestSuggestions];
+- (void)webContentsFinishedLoading {
+  if (webContents_->IsFocusedElementEditable()) {
+    // TODO(tnijssen): Actually request the current text and cursor position.
+    [self setText:@""];
+    selectionRange_ = NSMakeRange(0, 0);
+    editingWordRange_ = NSMakeRange(0, 0);
+    [self requestSuggestionsForText:@"" inRange:NSMakeRange(0, 0)];
   }
 }
 
@@ -198,11 +204,11 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   return NSMakeRange(location, length);
 }
 
-- (void)requestSuggestions {
+- (void)requestSuggestionsForText:(NSString*)text inRange:(NSRange)range {
   NSSpellChecker* spell_checker = [NSSpellChecker sharedSpellChecker];
   [spell_checker
-      requestCandidatesForSelectedRange:selectionRange_
-                               inString:text_
+      requestCandidatesForSelectedRange:range
+                               inString:text
                                   types:NSTextCheckingAllSystemTypes
                                 options:nil
                  inSpellDocumentWithTag:0
@@ -248,34 +254,21 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 }
 
 - (void)setWebContents:(content::WebContents*)webContents {
+  // TODO(tnijssen): Update the text suggestions when we change web contents.
   webContents_ = webContents;
   observer_->UpdateWebContents(webContents);
-
-  if (![self isTextfieldFocused]) {
-    [controller_ invalidateTouchBar];
-    return;
-  }
-
-  const base::string16 text =
-      webContents_->GetTopLevelRenderWidgetHostView()->GetSurroundingText();
-  const gfx::Range range =
-      webContents_->GetTopLevelRenderWidgetHostView()->GetSelectedRange();
-  if (range.IsValid())
-    [self updateTextSelection:text range:range];
-  else
-    [self updateTextSelection:base::string16() range:gfx::Range()];
 }
 
 - (content::WebContents*)webContents {
   return webContents_;
 }
 
-- (void)setSelectionRange:(const gfx::Range&)range {
-  selectionRange_ = range.ToNSRange();
+- (void)setSelectionRange:(NSRange)range {
+  selectionRange_ = range;
 }
 
-- (gfx::Range)selectionRange {
-  return gfx::Range(selectionRange_);
+- (NSRange)selectionRange {
+  return selectionRange_;
 }
 
 @end

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

@@ -52,8 +52,8 @@
 
 @implementation MockSuggestedTextTouchBarController
 
-- (void)requestSuggestions {
-  [self setSuggestions:@[ [self text] ]];
+- (void)requestSuggestionsForText:(NSString*)text inRange:(NSRange)range {
+  [self setSuggestions:@[ text ]];
   [[self controller] invalidateTouchBar];
 }
 
@@ -65,6 +65,10 @@
 
 namespace {
 
+NSString* const kSuggestedTextTouchBarId = @"suggested-text";
+NSString* const kText = @"text";
+NSString* const kEmptyText = @"";
+
 class SuggestedTextTouchBarControllerBrowserTest : public InProcessBrowserTest {
  public:
   void SetUp() override {
@@ -76,8 +80,10 @@ class SuggestedTextTouchBarControllerBrowserTest : public InProcessBrowserTest {
     web_textfield_controller_.reset(
         [[MockWebTextfieldTouchBarController alloc] init]);
     [web_textfield_controller_ resetNumInvalidations];
+    content::WebContents* web_contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
     touch_bar_controller_.reset([[MockSuggestedTextTouchBarController alloc]
-        initWithWebContents:GetActiveWebContents()
+        initWithWebContents:web_contents
                  controller:web_textfield_controller_]);
   }
 
@@ -92,10 +98,15 @@ class SuggestedTextTouchBarControllerBrowserTest : public InProcessBrowserTest {
         browser(), GURL("data:text/html;charset=utf-8,<input type=\"text\">"));
   }
 
-  content::WebContents* GetActiveWebContents() {
-    return browser()->tab_strip_model()->GetActiveWebContents();
+  MockWebTextfieldTouchBarController* web_textfield_controller() const {
+    return web_textfield_controller_;
+  }
+
+  MockSuggestedTextTouchBarController* touch_bar_controller() const {
+    return touch_bar_controller_;
   }
 
+ private:
   base::scoped_nsobject<MockWebTextfieldTouchBarController>
       web_textfield_controller_;
   base::scoped_nsobject<MockSuggestedTextTouchBarController>
@@ -107,15 +118,13 @@ class SuggestedTextTouchBarControllerBrowserTest : public InProcessBrowserTest {
 IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
                        TouchBarTest) {
   if (@available(macOS 10.12.2, *)) {
-    NSString* const kSuggestedTextTouchBarId = @"suggested-text";
-
     // Touch bar shouldn't appear if the focused element is not a textfield.
     UnfocusTextfield();
-    EXPECT_FALSE([touch_bar_controller_ makeTouchBar]);
+    EXPECT_FALSE([touch_bar_controller() makeTouchBar]);
 
     // Touch bar should appear if a textfield is focused.
     FocusTextfield();
-    NSTouchBar* touch_bar = [touch_bar_controller_ makeTouchBar];
+    NSTouchBar* touch_bar = [touch_bar_controller() makeTouchBar];
     EXPECT_TRUE(touch_bar);
     EXPECT_TRUE([[touch_bar customizationIdentifier]
         isEqual:ui::GetTouchBarId(kSuggestedTextTouchBarId)]);
@@ -124,28 +133,27 @@ IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
 
 // Tests that a change in text selection is handled properly.
 IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
-                       UpdateTextSelectionTest) {
+                       TextSelectionChangedTest) {
   if (@available(macOS 10.12.2, *)) {
-    NSString* const kText = @"text";
-    NSString* const kEmptyText = @"";
-    const gfx::Range kRange = gfx::Range(0, 4);
-    const gfx::Range kEmptyRange = gfx::Range();
+    const NSRange kRange = NSMakeRange(0, [kText length]);
+    const NSRange kEmptyRange = NSMakeRange(0, 0);
 
     // If not in a textfield,
     //  1. Textfield text should not be saved.
     //  2. Selected range should not be saved.
     //  3. Touch bar should be invalidated.
     UnfocusTextfield();
-    [touch_bar_controller_ setText:kEmptyText];
-    [touch_bar_controller_ setSelectionRange:kEmptyRange];
-    [web_textfield_controller_ resetNumInvalidations];
+    [touch_bar_controller() setText:kEmptyText];
+    [touch_bar_controller() setSelectionRange:kEmptyRange];
+    [web_textfield_controller() resetNumInvalidations];
 
-    [touch_bar_controller_ updateTextSelection:base::SysNSStringToUTF16(kText)
+    [touch_bar_controller()
+        webContentsTextSelectionChanged:base::SysNSStringToUTF16(kText)
                                   range:kRange];
-    EXPECT_STREQ(kEmptyText.UTF8String,
-                 [touch_bar_controller_ text].UTF8String);
-    EXPECT_EQ(kEmptyRange, [touch_bar_controller_ selectionRange]);
-    EXPECT_EQ(1, [web_textfield_controller_ numInvalidations]);
+    EXPECT_TRUE([kEmptyText isEqualToString:[touch_bar_controller() text]]);
+    EXPECT_EQ(gfx::Range(kEmptyRange),
+              gfx::Range([touch_bar_controller() selectionRange]));
+    EXPECT_EQ(1, [web_textfield_controller() numInvalidations]);
 
     // If in a textfield,
     //   1. Textfield text should be saved.
@@ -153,39 +161,20 @@ IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
     //   3. Suggestions should be generated based on text selection.
     //   4. Touch bar should be invalidated.
     FocusTextfield();
-    [touch_bar_controller_ setText:kEmptyText];
-    [touch_bar_controller_ setSelectionRange:kEmptyRange];
-    [web_textfield_controller_ resetNumInvalidations];
+    [touch_bar_controller() setText:kEmptyText];
+    [touch_bar_controller() setSelectionRange:kEmptyRange];
+    [web_textfield_controller() resetNumInvalidations];
 
-    [touch_bar_controller_ updateTextSelection:base::SysNSStringToUTF16(kText)
+    [touch_bar_controller()
+        webContentsTextSelectionChanged:base::SysNSStringToUTF16(kText)
                                   range:kRange];
-    EXPECT_STREQ(kText.UTF8String, [touch_bar_controller_ text].UTF8String);
-    EXPECT_EQ(kRange, [touch_bar_controller_ selectionRange]);
-    EXPECT_STREQ(kText.UTF8String,
-                 [touch_bar_controller_ firstSuggestion].UTF8String);
-    EXPECT_EQ(1, [web_textfield_controller_ numInvalidations]);
+    EXPECT_TRUE([kText isEqualToString:[touch_bar_controller() text]]);
+    EXPECT_EQ(gfx::Range(kRange),
+              gfx::Range([touch_bar_controller() selectionRange]));
+    EXPECT_TRUE(
+        [kText isEqualToString:[touch_bar_controller() firstSuggestion]]);
+    EXPECT_EQ(1, [web_textfield_controller() numInvalidations]);
   }
 }
 
-// Tests that a change in WebContents is handled properly.
-IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
-                       SetWebContentsTest) {
-  NSString* const kText = @"text";
-  const gfx::Range kRange = gfx::Range(1, 1);
-
-  // A null-pointer should not break the controller.
-  [touch_bar_controller_ setWebContents:nullptr];
-  EXPECT_FALSE([touch_bar_controller_ webContents]);
-
-  FocusTextfield();
-  [touch_bar_controller_ setText:kText];
-  [touch_bar_controller_ setSelectionRange:kRange];
-
-  // The text selection should change if the WebContents pointer is not null.
-  [touch_bar_controller_ setWebContents:GetActiveWebContents()];
-  EXPECT_EQ(GetActiveWebContents(), [touch_bar_controller_ webContents]);
-  EXPECT_STRNE(kText.UTF8String, [touch_bar_controller_ text].UTF8String);
-  EXPECT_NE(kRange, [touch_bar_controller_ selectionRange]);
-}
-
 }  // namespace
\ No newline at end of file

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

@@ -39,11 +39,11 @@ TEST_F(SuggestedTextTouchBarControllerUnitTest, CollapsedCandidateListTest) {
   if (@available(macOS 10.12.2, *)) {
     base::scoped_nsobject<NSCandidateListTouchBarItem> item;
 
-    [controller_ setSelectionRange:gfx::Range()];
+    [controller_ setSelectionRange:NSMakeRange(0, 0)];
     item.reset([controller_ createCandidateListItem]);
     EXPECT_FALSE([item isCollapsed]);
 
-    [controller_ setSelectionRange:gfx::Range(0, 1)];
+    [controller_ setSelectionRange:NSMakeRange(0, 1)];
     item.reset([controller_ createCandidateListItem]);
     EXPECT_TRUE([item isCollapsed]);
   }