Skip to content

Commit

Permalink
Cache result of ToolbarModelImpl#extractSearchTermsFromUrl.
Browse files Browse the repository at this point in the history
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 <thildebr@chromium.org>
Reviewed-by: Maria Khomenko <mariakhomenko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546258}
  • Loading branch information
Troy Hildebrandt authored and Commit Bot committed Mar 27, 2018
1 parent 6a0d250 commit e1c4a57
Showing 1 changed file with 33 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -365,14 +371,40 @@ 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
* 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 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);
Expand Down

0 comments on commit e1c4a57

Please sign in to comment.