[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/1153728Reviewed-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}

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

@@ -19,6 +19,10 @@ namespace content {
 class WebContents;
 }  // namespace content
 
+namespace gfx {
+class Range;
+}  // namespace gfx
+
 @interface SuggestedTextTouchBarController
     : NSObject<NSTouchBarDelegate, NSCandidateListTouchBarItemDelegate>
 
@@ -40,19 +44,15 @@ class WebContents;
      endSelectingCandidateAtIndex:(NSInteger)index
     API_AVAILABLE(macos(10.12.2));
 
-- (void)webContentsTextSelectionChanged:(const base::string16&)text
-                                  range:(NSRange)range
-    API_AVAILABLE(macos(10.12.2));
-
-- (void)webContentsFinishedLoading API_AVAILABLE(macos(10.12.2));
+- (void)updateTextSelection:(const base::string16&)text
+                      range:(const gfx::Range&)range;
 
 // 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)requestSuggestionsForText:(NSString*)text
-                          inRange:(NSRange)range API_AVAILABLE(macos(10.12.2));
+- (void)requestSuggestions 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 WebContents;
 - (WebTextfieldTouchBarController*)controller;
 - (void)setWebContents:(content::WebContents*)webContents;
 - (content::WebContents*)webContents;
-- (void)setSelectionRange:(NSRange)range;
-- (NSRange)selectionRange;
+- (void)setSelectionRange:(const gfx::Range&)range;
+- (gfx::Range)selectionRange;
 
 @end
 

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

@@ -6,6 +6,7 @@
 
 #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"
@@ -32,16 +33,12 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 
   void DidChangeTextSelection(const base::string16& text,
                               const gfx::Range& range) override {
-    if (@available(macOS 10.12.2, *)) {
-      [owner_ webContentsTextSelectionChanged:text range:range.ToNSRange()];
-    }
+    [owner_ updateTextSelection:text range:range];
   }
 
   void DidFinishLoad(content::RenderFrameHost* render_frame_host,
                      const GURL& validated_url) override {
-    if (@available(macOS 10.12.2, *)) {
-      [owner_ webContentsFinishedLoading];
-    }
+    [owner_ updateTextSelection:base::string16() range:gfx::Range()];
   }
 
  private:
@@ -80,6 +77,10 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 
 @implementation SuggestedTextTouchBarController
 
+- (BOOL)isTextfieldFocused {
+  return webContents_ && webContents_->IsFocusedElementEditable();
+}
+
 - (instancetype)initWithWebContents:(content::WebContents*)webContents
                          controller:
                              (WebTextfieldTouchBarController*)controller {
@@ -94,7 +95,7 @@ class WebContentsTextObserver : public content::WebContentsObserver {
 }
 
 - (NSTouchBar*)makeTouchBar {
-  if (!webContents_ || !webContents_->IsFocusedElementEditable())
+  if (![self isTextfieldFocused])
     return nil;
 
   base::scoped_nsobject<NSTouchBar> touchBar([[ui::NSTouchBar() alloc] init]);
@@ -143,26 +144,19 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   ui::LogTouchBarUMA(ui::TouchBarAction::TEXT_SUGGESTION);
 }
 
-- (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 {
+- (void)updateTextSelection:(const base::string16&)text
+                      range:(const gfx::Range&)range {
+  if (@available(macOS 10.12.2, *)) {
+    if (![self isTextfieldFocused]) {
       [controller_ invalidateTouchBar];
+      return;
     }
-}
 
-- (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)];
+    text_.reset([base::SysUTF16ToNSString(text) retain]);
+    selectionRange_ = range.ToNSRange();
+    editingWordRange_ =
+        [self editingWordRangeFromText:text cursorPosition:range.start()];
+    [self requestSuggestions];
   }
 }
 
@@ -204,11 +198,11 @@ class WebContentsTextObserver : public content::WebContentsObserver {
   return NSMakeRange(location, length);
 }
 
-- (void)requestSuggestionsForText:(NSString*)text inRange:(NSRange)range {
+- (void)requestSuggestions {
   NSSpellChecker* spell_checker = [NSSpellChecker sharedSpellChecker];
   [spell_checker
-      requestCandidatesForSelectedRange:range
-                               inString:text
+      requestCandidatesForSelectedRange:selectionRange_
+                               inString:text_
                                   types:NSTextCheckingAllSystemTypes
                                 options:nil
                  inSpellDocumentWithTag:0
@@ -254,21 +248,34 @@ 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:(NSRange)range {
-  selectionRange_ = range;
+- (void)setSelectionRange:(const gfx::Range&)range {
+  selectionRange_ = range.ToNSRange();
 }
 
-- (NSRange)selectionRange {
-  return selectionRange_;
+- (gfx::Range)selectionRange {
+  return gfx::Range(selectionRange_);
 }
 
 @end

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

@@ -52,8 +52,8 @@
 
 @implementation MockSuggestedTextTouchBarController
 
-- (void)requestSuggestionsForText:(NSString*)text inRange:(NSRange)range {
-  [self setSuggestions:@[ text ]];
+- (void)requestSuggestions {
+  [self setSuggestions:@[ [self text] ]];
   [[self controller] invalidateTouchBar];
 }
 
@@ -65,10 +65,6 @@
 
 namespace {
 
-NSString* const kSuggestedTextTouchBarId = @"suggested-text";
-NSString* const kText = @"text";
-NSString* const kEmptyText = @"";
-
 class SuggestedTextTouchBarControllerBrowserTest : public InProcessBrowserTest {
  public:
   void SetUp() override {
@@ -80,10 +76,8 @@ 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:web_contents
+        initWithWebContents:GetActiveWebContents()
                  controller:web_textfield_controller_]);
   }
 
@@ -98,15 +92,10 @@ class SuggestedTextTouchBarControllerBrowserTest : public InProcessBrowserTest {
         browser(), GURL("data:text/html;charset=utf-8,<input type=\"text\">"));
   }
 
-  MockWebTextfieldTouchBarController* web_textfield_controller() const {
-    return web_textfield_controller_;
-  }
-
-  MockSuggestedTextTouchBarController* touch_bar_controller() const {
-    return touch_bar_controller_;
+  content::WebContents* GetActiveWebContents() {
+    return browser()->tab_strip_model()->GetActiveWebContents();
   }
 
- private:
   base::scoped_nsobject<MockWebTextfieldTouchBarController>
       web_textfield_controller_;
   base::scoped_nsobject<MockSuggestedTextTouchBarController>
@@ -118,13 +107,15 @@ 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)]);
@@ -133,27 +124,28 @@ IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
 
 // Tests that a change in text selection is handled properly.
 IN_PROC_BROWSER_TEST_F(SuggestedTextTouchBarControllerBrowserTest,
-                       TextSelectionChangedTest) {
+                       UpdateTextSelectionTest) {
   if (@available(macOS 10.12.2, *)) {
-    const NSRange kRange = NSMakeRange(0, [kText length]);
-    const NSRange kEmptyRange = NSMakeRange(0, 0);
+    NSString* const kText = @"text";
+    NSString* const kEmptyText = @"";
+    const gfx::Range kRange = gfx::Range(0, 4);
+    const gfx::Range kEmptyRange = gfx::Range();
 
     // 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()
-        webContentsTextSelectionChanged:base::SysNSStringToUTF16(kText)
+    [touch_bar_controller_ updateTextSelection:base::SysNSStringToUTF16(kText)
                                          range:kRange];
-    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]);
+    EXPECT_STREQ(kEmptyText.UTF8String,
+                 [touch_bar_controller_ text].UTF8String);
+    EXPECT_EQ(kEmptyRange, [touch_bar_controller_ selectionRange]);
+    EXPECT_EQ(1, [web_textfield_controller_ numInvalidations]);
 
     // If in a textfield,
     //   1. Textfield text should be saved.
@@ -161,20 +153,39 @@ 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()
-        webContentsTextSelectionChanged:base::SysNSStringToUTF16(kText)
+    [touch_bar_controller_ updateTextSelection:base::SysNSStringToUTF16(kText)
                                          range:kRange];
-    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]);
+    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]);
   }
 }
 
+// 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:NSMakeRange(0, 0)];
+    [controller_ setSelectionRange:gfx::Range()];
     item.reset([controller_ createCandidateListItem]);
     EXPECT_FALSE([item isCollapsed]);
 
-    [controller_ setSelectionRange:NSMakeRange(0, 1)];
+    [controller_ setSelectionRange:gfx::Range(0, 1)];
     item.reset([controller_ createCandidateListItem]);
     EXPECT_TRUE([item isCollapsed]);
   }