From e1c4a571cc30435b7da908f3ed151e5f734406e8 Mon Sep 17 00:00:00 2001 From: Troy Hildebrandt Date: Tue, 27 Mar 2018 21:07:01 +0000 Subject: [PATCH] Cache result of ToolbarModelImpl#extractSearchTermsFromUrl. The call to #extractSearchTermsFromUrl is quite expensive and can happen for the same URL needlessly and endlessly right now. It's called 3 times for every omnibox focus, and 3 times for every unfocus for instance. Since the URL we're checking is often the same as previously, we cache the last URL and security level we saw when performing this call, and only perform the work if they differ, caching the values for use next time. This means that in most cases we only need to perform the work one time for every URL we navigate to. Bug: 825009 Change-Id: Ie28ef271afaf4813baf50c2f1e84d86e752565bc Reviewed-on: https://chromium-review.googlesource.com/980740 Commit-Queue: Troy Hildebrandt Reviewed-by: Maria Khomenko Cr-Commit-Position: refs/heads/master@{#546258} --- .../browser/toolbar/ToolbarModelImpl.java | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java index 05e7c455d743b9..dab5c22ed72555 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/toolbar/ToolbarModelImpl.java @@ -44,7 +44,13 @@ public class ToolbarModelImpl private boolean mIsIncognito; private int mPrimaryColor; private boolean mIsUsingBrandColor; + + // Query in Omnibox cached values to avoid unnecessary expensive calls. private boolean mQueryInOmniboxEnabled; + private String mPreviousUrl; + @ConnectionSecurityLevel + private int mPreviousSecurityLevel; + private String mCachedSearchTerms; /** * Default constructor for this class. @@ -365,7 +371,8 @@ private boolean securityLevelSafeForQueryInOmnibox() { } /** - * Extracts query terms from a URL if it's a SRP URL from the default search engine. + * Extracts query terms from a URL if it's a SRP URL from the default search engine, caching + * the result of the more expensive call to {@link #extractSearchTermsFromUrlInternal}. * * @param url The URL to extract search terms from. * @return The search terms. Returns null if not a DSE SRP URL or there are no search terms to @@ -373,6 +380,31 @@ private boolean securityLevelSafeForQueryInOmnibox() { * display search terms in place of SRP URL. */ private String extractSearchTermsFromUrl(String url) { + if (url == null) { + mCachedSearchTerms = null; + } else { + @ConnectionSecurityLevel + int securityLevel = getSecurityLevel(); + if (!url.equals(mPreviousUrl) || securityLevel != mPreviousSecurityLevel) { + mCachedSearchTerms = extractSearchTermsFromUrlInternal(url); + mPreviousUrl = url; + mPreviousSecurityLevel = securityLevel; + } + } + return mCachedSearchTerms; + } + + /** + * Extracts query terms from a URL if it's a SRP URL from the default search engine, returning + * the cached result of the previous call if this call is being performed for the same URL and + * security level as last time. + * + * @param url The URL to extract search terms from. + * @return The search terms. Returns null if not a DSE SRP URL or there are no search terms to + * extract, if query in omnibox is disabled, or if the security level is insufficient to + * display search terms in place of SRP URL. + */ + private String extractSearchTermsFromUrlInternal(String url) { if (!mQueryInOmniboxEnabled || !securityLevelSafeForQueryInOmnibox()) return null; String searchTerms = TemplateUrlService.getInstance().extractSearchTermsFromUrl(url);