[omnibox] Truncate clipboard text for the omnibox.

When the clipboard contains large text, certain user interactions with
the omnibox (e.g. right click) exhibit a significant delay. Previously,
when the clipboard contained more than 32*1024 characters, we skipped
classifying the text and disabled the 'Paste and Go / Search' context
menu item as those were the largest sources of delay. However, at
around 130 million characters, sanitizing the text begins to contribute
significant delay (3-4 seconds) as well. Additionally, certain user
actions fetch and sanitize the clipboard text multiple times; e.g.,
right clicking the omnibox fetches the clipboard 5 times; then
selecting 'paste' fetches the clipboard 2 additional times; these calls
accumulate an ~18 second delay.

This CL: (1) removes the earlier truncations and (2) instead truncates
the clipboard text when it is fetched, and therefore before it is
sanitized. Additionally, (3) it increased the truncation length to
url::kMaxURLChars (2*1024*1024) characters. This is sufficient to keep
the right click delay under 9 seconds while also leaving greater safe
room for potentially long URLs (google images serves data URLs up to at
least 30k characters).

Bug: 543675
Change-Id: I8c3df3dc262747d57e37cd766d84efa12d8d94ee
Reviewed-on: https://chromium-review.googlesource.com/c/1474192
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Tommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632813}

chrome/browser/ui/omnibox/clipboard_utils.cc

@@ -16,6 +16,7 @@ base::string16 GetClipboardText() {
                                    ui::CLIPBOARD_TYPE_COPY_PASTE)) {
     base::string16 text;
     clipboard->ReadText(ui::CLIPBOARD_TYPE_COPY_PASTE, &text);
+    text = text.substr(0, kMaxClipboardTextLength);
     return OmniboxView::SanitizeTextForPaste(text);
   }
 

chrome/browser/ui/omnibox/clipboard_utils.h

@@ -9,6 +9,18 @@
 
 #include "base/strings/string16.h"
 
+// Truncates the clipboard text returned in order to improve performance and
+// prevent unresponsiveness. For reference, a book is about ~500k characters and
+// data URLs served by google images are usually 30k characters or less.
+// We don't use url::kMaxURLChars (2M), as it's too large; it adds 2s+ delays
+// when right clicking the omnibox for clipboards larger than 2M. Additionally,
+// a 500k limit also allows us to not have to worry about length when
+// classifying the text: OmniboxViewViews::GetLabelForCommandId and
+// OmniboxEditModel::CanPasteAndGo. If we used a larger limit here (e.g. 2M),
+// then we'd need to limit the text to ~500k later anyways when classifying,
+// because classifying text longer than 500k adds a ~1s delays.
+static const size_t kMaxClipboardTextLength = 500 * 1024;
+
 // Returns the current clipboard contents as a string that can be pasted in.
 // In addition to just getting CF_UNICODETEXT out, this can also extract URLs
 // from bookmarks on the clipboard.

chrome/browser/ui/omnibox/clipboard_utils_unittest.cc

@@ -88,4 +88,22 @@ TEST_F(ClipboardUtilsTest, GetClipboardText) {
   EXPECT_TRUE(GetClipboardText().empty());
 }
 
+TEST_F(ClipboardUtilsTest, TruncateLongText) {
+  const base::string16 almost_long_text =
+      base::ASCIIToUTF16(std::string(kMaxClipboardTextLength, '.'));
+  {
+    ui::ScopedClipboardWriter clipboard_writer(ui::CLIPBOARD_TYPE_COPY_PASTE);
+    clipboard_writer.WriteText(almost_long_text);
+  }
+  EXPECT_EQ(almost_long_text, GetClipboardText());
+
+  const base::string16 long_text =
+      base::ASCIIToUTF16(std::string(kMaxClipboardTextLength + 1, '.'));
+  {
+    ui::ScopedClipboardWriter clipboard_writer(ui::CLIPBOARD_TYPE_COPY_PASTE);
+    clipboard_writer.WriteText(long_text);
+  }
+  EXPECT_EQ(almost_long_text, GetClipboardText());
+}
+
 }  // namespace

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -975,16 +975,8 @@ base::string16 OmniboxViewViews::GetLabelForCommandId(int command_id) const {
   base::string16 selection_text = gfx::TruncateString(
       clipboard_text, kMaxSelectionTextLength, gfx::WORD_BREAK);
 
-  // If the clipboard text is too long, this command will be disabled, so
-  // we skip the potentially expensive classification of the text and default to
-  // IDS_PASTE_AND_SEARCH.
-  bool paste_and_search =
-      clipboard_text.size() > OmniboxEditModel::kMaxPasteAndGoTextLength ||
-      model()->ClassifiesAsSearch(clipboard_text);
-
-  if (paste_and_search) {
+  if (model()->ClassifiesAsSearch(clipboard_text))
     return l10n_util::GetStringFUTF16(IDS_PASTE_AND_SEARCH, selection_text);
-  }
 
   // To ensure the search and url strings began to truncate at the exact same
   // number of characters, the pixel width at which the url begins to elide is

components/omnibox/browser/omnibox_edit_model.cc

@@ -533,9 +533,6 @@ bool OmniboxEditModel::CanPasteAndGo(const base::string16& text) const {
   if (!client_->IsPasteAndGoEnabled())
     return false;
 
-  if (text.length() > kMaxPasteAndGoTextLength)
-    return false;
-
   AutocompleteMatch match;
   ClassifyString(text, &match, nullptr);
   return match.destination_url.is_valid();

components/omnibox/browser/omnibox_edit_model.h

@@ -72,11 +72,6 @@ class OmniboxEditModel {
     DISALLOW_ASSIGN(State);
   };
 
-  // This is a mirror of content::kMaxURLDisplayChars because ios cannot depend
-  // on content. If clipboard contains more than kMaxPasteAndGoTextLength
-  // characters, then the paste & go option will be disabled.
-  static const size_t kMaxPasteAndGoTextLength = 32 * 1024;
-
   OmniboxEditModel(OmniboxView* view,
                    OmniboxEditController* controller,
                    std::unique_ptr<OmniboxClient> client);

components/omnibox/browser/omnibox_edit_model_unittest.cc

@@ -391,19 +391,6 @@ TEST_F(OmniboxEditModelTest, UnelideDoesNothingWhenFullURLAlreadyShown) {
   EXPECT_TRUE(model()->ShouldShowCurrentPageIcon());
 }
 
-TEST_F(OmniboxEditModelTest, DisablePasteAndGoForLongTexts) {
-  EXPECT_TRUE(model()->OmniboxEditModel::CanPasteAndGo(
-      base::ASCIIToUTF16("short text")));
-
-  base::string16 almost_long_text = base::ASCIIToUTF16(
-      std::string(OmniboxEditModel::kMaxPasteAndGoTextLength, '.'));
-  EXPECT_TRUE(model()->OmniboxEditModel::CanPasteAndGo(almost_long_text));
-
-  base::string16 long_text = base::ASCIIToUTF16(
-      std::string(OmniboxEditModel::kMaxPasteAndGoTextLength + 1, '.'));
-  EXPECT_FALSE(model()->OmniboxEditModel::CanPasteAndGo(long_text));
-}
-
 // The tab-switching system sometimes focuses the Omnibox even if it was not
 // previously focused. In those cases, ignore the saved focus state.
 TEST_F(OmniboxEditModelTest, IgnoreInvalidSavedFocusStates) {