Commit 8991af40 authored by Min Qin's avatar Min Qin Committed by Commit Bot

Add a field to OmniboxLog to indicate whether a query is kicked off by query tiles

When a query is kicked off by tiles, user could modify it multiple
times, before hitting the enter button or clicking on a suggestion
to finish the query.
This CL adds a field in OmniboxLog to indicate whether the query
is kicked off by a tile. So that the field can be passed to
OmniboxEvent proto later on. To do this, whenever the omnibox is focused
or autocompletion flow starts, a flag is turned on to check whether
the focus or the flow is caused by query tiles. And the flag is later
passed  to OmniboxLog when a suggestion is selected.

BUG=1090911

Change-Id: If0d9eab92b3073e7d209359efc20544aa9691a0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229882Reviewed-by: default avatarShakti Sahu <shaktisahu@chromium.org>
Reviewed-by: default avatarEnder <ender@google.com>
Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776874}
parent 90f36b6d
......@@ -213,7 +213,8 @@ public class NewTabPage implements NativePage, InvalidationAwareThumbnailProvide
}
@Override
public void focusSearchBox(boolean beginVoiceSearch, String pastedText) {
public void focusSearchBox(
boolean beginVoiceSearch, String pastedText, boolean fromQueryTile) {
if (mIsDestroyed) return;
if (VrModuleProvider.getDelegate().isInVr()) return;
if (mVoiceRecognitionHandler != null && beginVoiceSearch) {
......@@ -221,8 +222,10 @@ public class NewTabPage implements NativePage, InvalidationAwareThumbnailProvide
VoiceRecognitionHandler.VoiceInteractionSource.NTP);
} else if (mFakeboxDelegate != null) {
mFakeboxDelegate.setUrlBarFocus(true, pastedText,
pastedText == null ? OmniboxFocusReason.FAKE_BOX_TAP
: OmniboxFocusReason.FAKE_BOX_LONG_PRESS);
pastedText == null
? OmniboxFocusReason.FAKE_BOX_TAP
: (fromQueryTile ? OmniboxFocusReason.QUERY_TILES_NTP_TAP
: OmniboxFocusReason.FAKE_BOX_LONG_PRESS));
}
}
......
......@@ -310,7 +310,8 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
private void initializeSearchBoxTextView() {
TraceEvent.begin(TAG + ".initializeSearchBoxTextView()");
mSearchBoxCoordinator.setSearchBoxClickListener(v -> mManager.focusSearchBox(false, null));
mSearchBoxCoordinator.setSearchBoxClickListener(
v -> mManager.focusSearchBox(false, null, false));
mSearchBoxCoordinator.setSearchBoxTextWatcher(new TextWatcher() {
@Override
public void beforeTextChanged(CharSequence s, int start, int count, int after) {}
......@@ -321,8 +322,9 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
@Override
public void afterTextChanged(Editable s) {
if (s.length() == 0) return;
mManager.focusSearchBox(false, s.toString());
mSearchBoxCoordinator.setSearchText("");
mManager.focusSearchBox(
false, s.toString(), mSearchBoxCoordinator.isTextChangeFromTiles());
mSearchBoxCoordinator.setSearchText("", false);
}
});
TraceEvent.end(TAG + ".initializeSearchBoxTextView()");
......@@ -331,7 +333,7 @@ public class NewTabPageLayout extends LinearLayout implements TileGroup.Observer
private void initializeVoiceSearchButton() {
TraceEvent.begin(TAG + ".initializeVoiceSearchButton()");
mSearchBoxCoordinator.addVoiceSearchButtonClickListener(
v -> mManager.focusSearchBox(true, null));
v -> mManager.focusSearchBox(true, null, false));
if (SearchEngineLogoUtils.isSearchEngineLogoEnabled()) {
// View is 48dp, image is 24dp. Increasing the padding from 4dp -> 8dp will split the
// remaining 16dp evenly between start/end resulting in a paddingEnd of 8dp.
......
......@@ -21,8 +21,9 @@ public interface NewTabPageManager extends SuggestionsUiDelegate {
* Animates the search box up into the omnibox and bring up the keyboard.
* @param beginVoiceSearch Whether to begin a voice search.
* @param pastedText Text to paste in the omnibox after it's been focused. May be null.
* @param fromQueryTiles Whether the search box focus is caused by clicking on Query Tiles.
*/
void focusSearchBox(boolean beginVoiceSearch, String pastedText);
void focusSearchBox(boolean beginVoiceSearch, String pastedText, boolean fromQueryTiles);
/**
* Performs a search query on the current {@link Tab}.
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.ntp.search;
import android.content.Context;
import android.graphics.drawable.Drawable;
import android.text.TextWatcher;
import android.util.Pair;
import android.view.View;
import android.view.View.OnClickListener;
import android.view.ViewGroup;
......@@ -56,8 +57,8 @@ public class SearchBoxCoordinator {
mModel.set(SearchBoxProperties.VISIBILITY, visible);
}
public void setSearchText(String text) {
mModel.set(SearchBoxProperties.SEARCH_TEXT, text);
public void setSearchText(String text, boolean fromQueryTiles) {
mModel.set(SearchBoxProperties.SEARCH_TEXT, Pair.create(text, fromQueryTiles));
}
public void setSearchBoxClickListener(OnClickListener listener) {
......@@ -87,4 +88,9 @@ public class SearchBoxCoordinator {
public void setChipDelegate(SearchBoxChipDelegate chipDelegate) {
mMediator.setChipDelegate(chipDelegate);
}
public boolean isTextChangeFromTiles() {
Pair<String, Boolean> searchText = mModel.get(SearchBoxProperties.SEARCH_TEXT);
return searchText == null ? false : searchText.second;
}
}
......@@ -9,6 +9,7 @@ import android.content.res.ColorStateList;
import android.graphics.Bitmap;
import android.graphics.drawable.Drawable;
import android.text.TextUtils;
import android.util.Pair;
import android.view.View.OnClickListener;
import android.view.ViewGroup;
......@@ -45,7 +46,6 @@ class SearchBoxMediator
mContext = context;
mModel = model;
mView = view;
PropertyModelChangeProcessor.create(mModel, mView, new SearchBoxViewBinder());
}
......@@ -106,7 +106,7 @@ class SearchBoxMediator
boolean isChipVisible = mModel.get(SearchBoxProperties.CHIP_VISIBILITY);
if (isChipVisible) {
String chipText = mModel.get(SearchBoxProperties.CHIP_TEXT);
mModel.set(SearchBoxProperties.SEARCH_TEXT, chipText);
mModel.set(SearchBoxProperties.SEARCH_TEXT, Pair.create(chipText, true));
mChipDelegate.onCancelClicked();
}
listener.onClick(v);
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.ntp.search;
import android.content.res.ColorStateList;
import android.graphics.drawable.Drawable;
import android.text.TextWatcher;
import android.util.Pair;
import android.view.View.OnClickListener;
import org.chromium.ui.modelutil.PropertyKey;
......@@ -28,7 +29,8 @@ interface SearchBoxProperties {
new WritableObjectPropertyKey<>();
WritableObjectPropertyKey<OnClickListener> VOICE_SEARCH_CLICK_CALLBACK =
new WritableObjectPropertyKey<>();
WritableObjectPropertyKey<String> SEARCH_TEXT = new WritableObjectPropertyKey<>();
WritableObjectPropertyKey<Pair<String, Boolean>> SEARCH_TEXT =
new WritableObjectPropertyKey<>();
WritableBooleanPropertyKey SEARCH_HINT_VISIBILITY = new WritableBooleanPropertyKey();
WritableObjectPropertyKey<OnClickListener> SEARCH_BOX_CLICK_CALLBACK =
new WritableObjectPropertyKey<>();
......
......@@ -56,7 +56,7 @@ class SearchBoxViewBinder
searchBoxTextView.addTextChangedListener(
model.get(SearchBoxProperties.SEARCH_BOX_TEXT_WATCHER));
} else if (SearchBoxProperties.SEARCH_TEXT == propertyKey) {
searchBoxTextView.setText(model.get(SearchBoxProperties.SEARCH_TEXT));
searchBoxTextView.setText(model.get(SearchBoxProperties.SEARCH_TEXT).first);
} else if (SearchBoxProperties.SEARCH_HINT_VISIBILITY == propertyKey) {
boolean isHintVisible = model.get(SearchBoxProperties.SEARCH_HINT_VISIBILITY);
searchBoxTextView.setHint(isHintVisible
......
......@@ -42,7 +42,7 @@ public interface LocationBar extends UrlBarDelegate, FakeboxDelegate {
OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_LONG_PRESS,
OmniboxFocusReason.DEFAULT_WITH_HARDWARE_KEYBOARD, OmniboxFocusReason.SEARCH_QUERY,
OmniboxFocusReason.LAUNCH_NEW_INCOGNITO_TAB, OmniboxFocusReason.MENU_OR_KEYBOARD_ACTION,
OmniboxFocusReason.UNFOCUS})
OmniboxFocusReason.UNFOCUS, OmniboxFocusReason.QUERY_TILES_NTP_TAP})
@Retention(RetentionPolicy.SOURCE)
public @interface OmniboxFocusReason {
int OMNIBOX_TAP = 0;
......@@ -59,7 +59,8 @@ public interface LocationBar extends UrlBarDelegate, FakeboxDelegate {
int LAUNCH_NEW_INCOGNITO_TAB = 10;
int MENU_OR_KEYBOARD_ACTION = 11;
int UNFOCUS = 12;
int NUM_ENTRIES = 13;
int QUERY_TILES_NTP_TAP = 13;
int NUM_ENTRIES = 14;
}
/**
......
......@@ -118,6 +118,7 @@ public class LocationBarLayout extends FrameLayout
protected boolean mNativeInitialized;
private boolean mUrlHasFocus;
private boolean mUrlFocusedFromFakebox;
private boolean mUrlFocusedFromQueryTiles;
private boolean mUrlFocusedWithoutAnimations;
protected boolean mVoiceSearchEnabled;
......@@ -495,6 +496,7 @@ public class LocationBarLayout extends FrameLayout
imm.viewClicked(mUrlBar);
} else {
mUrlFocusedFromFakebox = false;
mUrlFocusedFromQueryTiles = false;
mUrlFocusedWithoutAnimations = false;
// Focus change caused by a close-tab may result in an invalid current tab.
......@@ -563,6 +565,11 @@ public class LocationBarLayout extends FrameLayout
return mUrlFocusedFromFakebox;
}
@Override
public boolean didFocusUrlFromQueryTiles() {
return mUrlFocusedFromQueryTiles;
}
@Override
public void showUrlBarCursorWithoutFocusAnimations() {
if (mUrlHasFocus || mUrlFocusedFromFakebox) return;
......@@ -1033,7 +1040,6 @@ public class LocationBarLayout extends FrameLayout
boolean shouldBeFocused, @Nullable String pastedText, @OmniboxFocusReason int reason) {
if (shouldBeFocused) {
if (!mUrlHasFocus) recordOmniboxFocusReason(reason);
if (reason == LocationBar.OmniboxFocusReason.FAKE_BOX_TAP
|| reason == LocationBar.OmniboxFocusReason.FAKE_BOX_LONG_PRESS
|| reason == LocationBar.OmniboxFocusReason.TASKS_SURFACE_FAKE_BOX_LONG_PRESS
......@@ -1041,6 +1047,11 @@ public class LocationBarLayout extends FrameLayout
mUrlFocusedFromFakebox = true;
}
if (reason == LocationBar.OmniboxFocusReason.QUERY_TILES_NTP_TAP) {
mUrlFocusedFromFakebox = true;
mUrlFocusedFromQueryTiles = true;
}
if (mUrlHasFocus && mUrlFocusedWithoutAnimations) {
handleUrlFocusAnimation(mUrlHasFocus);
} else {
......
......@@ -104,9 +104,11 @@ public class AutocompleteController {
* not focused on the text.
* @param preventInlineAutocomplete Whether autocomplete suggestions should be prevented.
* @param queryTileId The ID of the query tile selected by the user, if any.
* @param isQueryStartedFromTiles Whether the search query is started from query tiles.
*/
public void start(Profile profile, String url, int pageClassification, String text,
int cursorPosition, boolean preventInlineAutocomplete, @Nullable String queryTileId) {
int cursorPosition, boolean preventInlineAutocomplete, @Nullable String queryTileId,
boolean isQueryStartedFromTiles) {
assert mListener != null : "Ensure a listener is set prior to calling.";
// crbug.com/764749
Log.w(TAG, "starting autocomplete controller..[%b][%b]", profile == null,
......@@ -119,7 +121,8 @@ public class AutocompleteController {
if (mNativeAutocompleteControllerAndroid != 0) {
AutocompleteControllerJni.get().start(mNativeAutocompleteControllerAndroid,
AutocompleteController.this, text, cursorPosition, null, url,
pageClassification, preventInlineAutocomplete, false, false, true, queryTileId);
pageClassification, preventInlineAutocomplete, false, false, true, queryTileId,
isQueryStartedFromTiles);
mWaitingForSuggestionsToCache = false;
}
}
......@@ -384,8 +387,8 @@ public class AutocompleteController {
void start(long nativeAutocompleteControllerAndroid, AutocompleteController caller,
String text, int cursorPosition, String desiredTld, String currentUrl,
int pageClassification, boolean preventInlineAutocomplete, boolean preferKeyword,
boolean allowExactKeywordMatch, boolean wantAsynchronousMatches,
String queryTileId);
boolean allowExactKeywordMatch, boolean wantAsynchronousMatches, String queryTileId,
boolean isQueryStartedFromTiles);
OmniboxSuggestion classify(long nativeAutocompleteControllerAndroid,
AutocompleteController caller, String text, boolean focusedFromFakebox);
void stop(long nativeAutocompleteControllerAndroid, AutocompleteController caller,
......
......@@ -65,4 +65,9 @@ public interface AutocompleteDelegate extends UrlBarDelegate {
* @return Whether the URL currently has focus.
*/
boolean isUrlBarFocused();
/**
* @return Whether the omnibox was focused because of tapping on query tiles.
*/
boolean didFocusUrlFromQueryTiles();
}
......@@ -556,7 +556,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
mDataProvider.getPageClassification(mDelegate.didFocusUrlFromFakebox()),
mUrlBarEditingTextProvider.getTextWithoutAutocomplete(),
mUrlBarEditingTextProvider.getSelectionStart(),
!mUrlBarEditingTextProvider.shouldAutocomplete(), queryTile.id);
!mUrlBarEditingTextProvider.shouldAutocomplete(), queryTile.id, true);
}
/**
......@@ -786,7 +786,8 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
int pageClassification =
mDataProvider.getPageClassification(mDelegate.didFocusUrlFromFakebox());
mAutocomplete.start(profile, mDataProvider.getCurrentUrl(), pageClassification,
textWithoutAutocomplete, cursorPosition, preventAutocomplete, null);
textWithoutAutocomplete, cursorPosition, preventAutocomplete, null,
mDelegate.didFocusUrlFromQueryTiles());
};
if (mNativeInitialized) {
mHandler.postDelayed(mRequestSuggestions, OMNIBOX_SUGGESTION_START_DELAY_MS);
......@@ -1209,7 +1210,7 @@ class AutocompleteMediator implements OnSuggestionsReceivedListener, StartStopWi
stopAutocomplete(false);
if (mDataProvider.hasTab()) {
mAutocomplete.start(mDataProvider.getProfile(), mDataProvider.getCurrentUrl(),
mDataProvider.getPageClassification(false), query, -1, false, null);
mDataProvider.getPageClassification(false), query, -1, false, null, false);
}
}
......
......@@ -121,7 +121,7 @@ public class QueryTileSection {
boolean isLastLevelTile = queryTile.children.isEmpty();
if (isLastLevelTile) {
if (QueryTileUtils.isQueryEditingEnabled()) {
mSearchBoxCoordinator.setSearchText(queryTile.queryText);
mSearchBoxCoordinator.setSearchText(queryTile.queryText, true);
} else {
mSubmitQueryCallback.onResult(
new QueryInfo(queryTile.queryText, queryTile.searchParams));
......
......@@ -595,7 +595,7 @@ public class AutocompleteMediatorUnitTest {
mMediator.onNativeInitialized();
mMediator.onTextChanged("test", "testing");
verify(mAutocompleteController)
.start(profile, url, pageClassification, "test", 4, false, null);
.start(profile, url, pageClassification, "test", 4, false, null, false);
}
@CalledByNativeJavaTest
......@@ -618,7 +618,7 @@ public class AutocompleteMediatorUnitTest {
mMediator.onTextChanged("test", "testing");
mMediator.onTextChanged("nottest", "nottesting");
verify(mAutocompleteController)
.start(profile, url, pageClassification, "nottest", 4, false, null);
.start(profile, url, pageClassification, "nottest", 4, false, null, false);
}
@CalledByNativeJavaTest
......
......@@ -176,7 +176,8 @@ void AutocompleteControllerAndroid::Start(
bool prefer_keyword,
bool allow_exact_keyword_match,
bool want_asynchronous_matches,
const JavaRef<jstring>& j_query_tile_id) {
const JavaRef<jstring>& j_query_tile_id,
bool is_query_started_from_tiles) {
if (!autocomplete_controller_)
return;
......@@ -199,6 +200,7 @@ void AutocompleteControllerAndroid::Start(
input_.set_want_asynchronous_matches(want_asynchronous_matches);
if (!j_query_tile_id.is_null())
input_.set_query_tile_id(ConvertJavaStringToUTF8(env, j_query_tile_id));
is_query_started_from_tiles_ = is_query_started_from_tiles;
autocomplete_controller_->Start(input_);
}
......@@ -243,6 +245,7 @@ void AutocompleteControllerAndroid::OnOmniboxFocused(
input_.set_current_url(current_url);
input_.set_current_title(current_title);
input_.set_from_omnibox_focus(true);
is_query_started_from_tiles_ = false;
autocomplete_controller_->Start(input_);
}
......@@ -315,6 +318,7 @@ void AutocompleteControllerAndroid::OnSuggestionSelected(
completed_length,
now - autocomplete_controller_->last_time_default_match_changed(),
autocomplete_controller_->result());
log.is_query_started_from_tile = is_query_started_from_tiles_;
autocomplete_controller_->AddProvidersInfo(&log.providers_info);
OmniboxEventGlobalTracker::GetInstance()->OnURLOpened(&log);
......@@ -618,7 +622,7 @@ AutocompleteControllerAndroid::GetTopSynchronousResult(
inside_synchronous_start_ = true;
Start(env, obj, j_text, -1, nullptr, nullptr, prevent_inline_autocomplete,
false, false, false, focused_from_fakebox, JavaRef<jstring>());
false, false, false, focused_from_fakebox, JavaRef<jstring>(), false);
inside_synchronous_start_ = false;
DCHECK(autocomplete_controller_->done());
const AutocompleteResult& result = autocomplete_controller_->result();
......
......@@ -43,7 +43,8 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
bool prefer_keyword,
bool allow_exact_keyword_match,
bool want_asynchronous_matches,
const base::android::JavaRef<jstring>& j_query_tile_id);
const base::android::JavaRef<jstring>& j_query_tile_id,
bool is_query_started_from_tiles);
base::android::ScopedJavaLocalRef<jobject> Classify(
JNIEnv* env,
const base::android::JavaParamRef<jobject>& obj,
......@@ -155,6 +156,10 @@ class AutocompleteControllerAndroid : public AutocompleteController::Observer,
JavaObjectWeakGlobalRef weak_java_autocomplete_controller_android_;
Profile* profile_;
// Whether the omnibox input is a query that starts building
// by clicking on an image tile.
bool is_query_started_from_tiles_ = false;
DISALLOW_COPY_AND_ASSIGN(AutocompleteControllerAndroid);
};
......
......@@ -133,7 +133,8 @@ public class OmniboxTestUtils {
@Override
public void start(Profile profile, String url, int pageClassification, final String text,
int cursorPosition, boolean preventInlineAutocomplete, String queryTileId) {
int cursorPosition, boolean preventInlineAutocomplete, String queryTileId,
boolean isQueryStartedFromTiles) {
mStartAutocompleteCalled = true;
mSuggestionsDispatcher = new Runnable() {
@Override
......@@ -194,7 +195,8 @@ public class OmniboxTestUtils {
@Override
public void start(Profile profile, String url, int pageClassification, String text,
int cursorPosition, boolean preventInlineAutocomplete, String queryTileId) {}
int cursorPosition, boolean preventInlineAutocomplete, String queryTileId,
boolean isQueryStartedFromTiles) {}
@Override
public void startZeroSuggest(Profile profile, String omniboxText, String url,
......
......@@ -110,6 +110,10 @@ struct OmniboxLog {
// AutocompleteController::AddProvidersInfo() and
// AutocompleteProvider::AddProviderInfo() above.
ProvidersInfo providers_info;
// Whether the omnibox input is a search query that is started
// by clicking on a image tile. Currently only used on Android.
bool is_query_started_from_tile = false;
};
#endif // COMPONENTS_OMNIBOX_BROWSER_OMNIBOX_LOG_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment