Commit ec2c17d7 authored by manuk's avatar manuk Committed by Commit Bot

[chrome://omnibox] Update result details for real omnibox inputs.

Above each response, the debugs page displays result details including:
1) cursor position
2) elapsed time
3) all providers done
4) host
5) has isTypedHost
These are updated when inputting into the HTML omnibox, but when inputting into
the window omnibox, only (3) is updated correctly.

This CL:
1) Updates the 5 fields when inputting from the window omnibox.
2) Adds 'input type' (e.g. URL).
3) Wires in cursor position similar to the other 4 fields from the page handler
and removes the hacked cursor position wired in from query inputs.
4) Updates the label for (5) to 'is typed host'.
5) Rearranges related variables to be consistent between files.

Bug: 1011632
Change-Id: I830b4af3d01d18faafbdaaeaea8614624a401c4c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1846311Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#704258}
parent 0d20421f
...@@ -268,10 +268,9 @@ ...@@ -268,10 +268,9 @@
<span class="label"> <span class="label">
all providers done = <span id="done"></span> all providers done = <span id="done"></span>
</span> </span>
<span class="label">type = <span id="type"></span></span>
<span class="label">host = <span id="host"></span></span> <span class="label">host = <span id="host"></span></span>
<span class="label"> <span class="label">is typed host = <span id="is-typed-host"></span></span>
has isTypedHost = <span id="is-typed-host"></span>
</span>
</template> </template>
<omnibox-input id="omnibox-input"></omnibox-input> <omnibox-input id="omnibox-input"></omnibox-input>
......
...@@ -158,7 +158,6 @@ document.addEventListener('DOMContentLoaded', () => { ...@@ -158,7 +158,6 @@ document.addEventListener('DOMContentLoaded', () => {
exportDelegate = new ExportDelegate(omniboxOutput, omniboxInput); exportDelegate = new ExportDelegate(omniboxOutput, omniboxInput);
omniboxInput.addEventListener('query-inputs-changed', e => { omniboxInput.addEventListener('query-inputs-changed', e => {
omniboxOutput.updateQueryInputs(e.detail);
browserProxy.makeRequest( browserProxy.makeRequest(
e.detail.inputText, e.detail.resetAutocompleteController, e.detail.inputText, e.detail.resetAutocompleteController,
e.detail.cursorPosition, e.detail.zeroSuggest, e.detail.cursorPosition, e.detail.zeroSuggest,
...@@ -211,7 +210,6 @@ class ExportDelegate { ...@@ -211,7 +210,6 @@ class ExportDelegate {
} }
this.omniboxInput_.queryInputs = importData.queryInputs; this.omniboxInput_.queryInputs = importData.queryInputs;
this.omniboxInput_.displayInputs = importData.displayInputs; this.omniboxInput_.displayInputs = importData.displayInputs;
this.omniboxOutput_.updateQueryInputs(importData.queryInputs);
this.omniboxOutput_.updateDisplayInputs(importData.displayInputs); this.omniboxOutput_.updateDisplayInputs(importData.displayInputs);
this.omniboxOutput_.setResponsesHistory(importData.responsesHistory); this.omniboxOutput_.setResponsesHistory(importData.responsesHistory);
return true; return true;
......
...@@ -8,6 +8,7 @@ cr.define('omnibox_output', function() { ...@@ -8,6 +8,7 @@ cr.define('omnibox_output', function() {
* cursorPosition: number, * cursorPosition: number,
* time: number, * time: number,
* done: boolean, * done: boolean,
* type: string,
* host: string, * host: string,
* isTypedHost: boolean, * isTypedHost: boolean,
* }} * }}
...@@ -31,19 +32,12 @@ cr.define('omnibox_output', function() { ...@@ -31,19 +32,12 @@ cr.define('omnibox_output', function() {
this.responsesHistory = []; this.responsesHistory = [];
/** @private {!Array<!OutputResultsGroup>} */ /** @private {!Array<!OutputResultsGroup>} */
this.resultsGroups_ = []; this.resultsGroups_ = [];
/** @private {!QueryInputs} */
this.queryInputs_ = /** @type {!QueryInputs} */ ({});
/** @private {!DisplayInputs} */ /** @private {!DisplayInputs} */
this.displayInputs_ = OmniboxInput.defaultDisplayInputs; this.displayInputs_ = OmniboxInput.defaultDisplayInputs;
/** @private {string} */ /** @private {string} */
this.filterText_ = ''; this.filterText_ = '';
} }
/** @param {!QueryInputs} queryInputs */
updateQueryInputs(queryInputs) {
this.queryInputs_ = queryInputs;
}
/** @param {!DisplayInputs} displayInputs */ /** @param {!DisplayInputs} displayInputs */
updateDisplayInputs(displayInputs) { updateDisplayInputs(displayInputs) {
this.displayInputs_ = displayInputs; this.displayInputs_ = displayInputs;
...@@ -103,8 +97,7 @@ cr.define('omnibox_output', function() { ...@@ -103,8 +97,7 @@ cr.define('omnibox_output', function() {
* @private @param {!mojom.OmniboxResponse} response * @private @param {!mojom.OmniboxResponse} response
*/ */
createResultsGroup_(response) { createResultsGroup_(response) {
const resultsGroup = const resultsGroup = OutputResultsGroup.create(response);
OutputResultsGroup.create(response, this.queryInputs_.cursorPosition);
this.resultsGroups_.push(resultsGroup); this.resultsGroups_.push(resultsGroup);
this.$$('#contents').appendChild(resultsGroup); this.$$('#contents').appendChild(resultsGroup);
...@@ -190,12 +183,11 @@ cr.define('omnibox_output', function() { ...@@ -190,12 +183,11 @@ cr.define('omnibox_output', function() {
class OutputResultsGroup extends OmniboxElement { class OutputResultsGroup extends OmniboxElement {
/** /**
* @param {!mojom.OmniboxResponse} resultsGroup * @param {!mojom.OmniboxResponse} resultsGroup
* @param {number} cursorPosition
* @return {!OutputResultsGroup} * @return {!OutputResultsGroup}
*/ */
static create(resultsGroup, cursorPosition) { static create(resultsGroup) {
const outputResultsGroup = new OutputResultsGroup(); const outputResultsGroup = new OutputResultsGroup();
outputResultsGroup.setResultsGroup(resultsGroup, cursorPosition); outputResultsGroup.setResultsGroup(resultsGroup);
return outputResultsGroup; return outputResultsGroup;
} }
...@@ -203,18 +195,16 @@ cr.define('omnibox_output', function() { ...@@ -203,18 +195,16 @@ cr.define('omnibox_output', function() {
super('output-results-group-template'); super('output-results-group-template');
} }
/** /** @param {!mojom.OmniboxResponse} resultsGroup */
* @param {!mojom.OmniboxResponse} resultsGroup setResultsGroup(resultsGroup) {
* @param {number} cursorPosition
*/
setResultsGroup(resultsGroup, cursorPosition) {
/** @private {ResultsDetails} */ /** @private {ResultsDetails} */
this.details_ = { this.details_ = {
cursorPosition: cursorPosition, cursorPosition: resultsGroup.cursorPosition,
time: resultsGroup.timeSinceOmniboxStartedMs, time: resultsGroup.timeSinceOmniboxStartedMs,
done: resultsGroup.done, done: resultsGroup.done,
type: resultsGroup.type,
host: resultsGroup.host, host: resultsGroup.host,
isTypedHost: resultsGroup.isTypedHost isTypedHost: resultsGroup.isTypedHost,
}; };
/** @type {!Array<!OutputHeader>} */ /** @type {!Array<!OutputHeader>} */
this.headers = COLUMNS.map(OutputHeader.create); this.headers = COLUMNS.map(OutputHeader.create);
...@@ -353,6 +343,7 @@ cr.define('omnibox_output', function() { ...@@ -353,6 +343,7 @@ cr.define('omnibox_output', function() {
this.$$('#cursor-position').textContent = details.cursorPosition; this.$$('#cursor-position').textContent = details.cursorPosition;
this.$$('#time').textContent = details.time; this.$$('#time').textContent = details.time;
this.$$('#done').textContent = details.done; this.$$('#done').textContent = details.done;
this.$$('#type').textContent = details.type;
this.$$('#host').textContent = details.host; this.$$('#host').textContent = details.host;
this.$$('#is-typed-host').textContent = details.isTypedHost; this.$$('#is-typed-host').textContent = details.isTypedHost;
} }
......
...@@ -52,9 +52,10 @@ struct AutocompleteResultsForProvider { ...@@ -52,9 +52,10 @@ struct AutocompleteResultsForProvider {
}; };
struct OmniboxResponse { struct OmniboxResponse {
bool done; int32 cursor_position;
// Time delta since the request was started, in milliseconds. // Time delta since the request was started, in milliseconds.
int32 time_since_omnibox_started_ms; int32 time_since_omnibox_started_ms;
bool done;
// The inferred metrics::OmniboxInputType of the request represented as a // The inferred metrics::OmniboxInputType of the request represented as a
// string. // string.
string type; string type;
......
...@@ -235,22 +235,25 @@ void OmniboxPageHandler::OnResultChanged(bool default_match_changed) { ...@@ -235,22 +235,25 @@ void OmniboxPageHandler::OnResultChanged(bool default_match_changed) {
} }
void OmniboxPageHandler::OnOmniboxQuery(AutocompleteController* controller, void OmniboxPageHandler::OnOmniboxQuery(AutocompleteController* controller,
const base::string16& input_text) { const AutocompleteInput& input) {
time_omnibox_started_ = base::Time::Now();
input_ = input;
page_->HandleNewAutocompleteQuery(controller == controller_.get(), page_->HandleNewAutocompleteQuery(controller == controller_.get(),
base::UTF16ToUTF8(input_text)); base::UTF16ToUTF8(input.text()));
} }
void OmniboxPageHandler::OnOmniboxResultChanged( void OmniboxPageHandler::OnOmniboxResultChanged(
bool default_match_changed, bool default_match_changed,
AutocompleteController* controller) { AutocompleteController* controller) {
mojom::OmniboxResponsePtr response(mojom::OmniboxResponse::New()); mojom::OmniboxResponsePtr response(mojom::OmniboxResponse::New());
response->done = controller->done(); response->cursor_position = input_.cursor_position();
response->time_since_omnibox_started_ms = response->time_since_omnibox_started_ms =
(base::Time::Now() - time_omnibox_started_).InMilliseconds(); (base::Time::Now() - time_omnibox_started_).InMilliseconds();
response->done = controller->done();
response->type = AutocompleteInput::TypeToString(input_.type());
const base::string16 host = const base::string16 host =
input_.text().substr(input_.parts().host.begin, input_.parts().host.len); input_.text().substr(input_.parts().host.begin, input_.parts().host.len);
response->host = base::UTF16ToUTF8(host); response->host = base::UTF16ToUTF8(host);
response->type = AutocompleteInput::TypeToString(input_.type());
bool is_typed_host; bool is_typed_host;
if (!LookupIsTypedHost(host, &is_typed_host)) if (!LookupIsTypedHost(host, &is_typed_host))
is_typed_host = false; is_typed_host = false;
...@@ -392,30 +395,22 @@ void OmniboxPageHandler::StartOmniboxQuery(const std::string& input_string, ...@@ -392,30 +395,22 @@ void OmniboxPageHandler::StartOmniboxQuery(const std::string& input_string,
// actual results to not depend on the state of the previous request. // actual results to not depend on the state of the previous request.
if (reset_autocomplete_controller) if (reset_autocomplete_controller)
ResetController(); ResetController();
// TODO (manukh): OmniboxPageHandler::StartOmniboxQuery is invoked only for AutocompleteInput input(
// queries from the debug page and not for queries from the browser omnibox.
// time_omnibox_started_ and input_ are therefore not set for browser omnibox
// queries, resulting in inaccurate time_since_omnibox_started_ms, host, type,
// and is_typed_host values in the result object being sent to the debug page.
// For the user, this means the 'details' section is mostly inaccurate for
// browser omnibox queries.
time_omnibox_started_ = base::Time::Now();
input_ = AutocompleteInput(
base::UTF8ToUTF16(input_string), cursor_position, base::UTF8ToUTF16(input_string), cursor_position,
static_cast<metrics::OmniboxEventProto::PageClassification>( static_cast<metrics::OmniboxEventProto::PageClassification>(
page_classification), page_classification),
ChromeAutocompleteSchemeClassifier(profile_)); ChromeAutocompleteSchemeClassifier(profile_));
GURL current_url_gurl{current_url}; GURL current_url_gurl{current_url};
if (current_url_gurl.is_valid()) if (current_url_gurl.is_valid())
input_.set_current_url(current_url_gurl); input.set_current_url(current_url_gurl);
input_.set_current_title(base::UTF8ToUTF16(current_url)); input.set_current_title(base::UTF8ToUTF16(current_url));
input_.set_prevent_inline_autocomplete(prevent_inline_autocomplete); input.set_prevent_inline_autocomplete(prevent_inline_autocomplete);
input_.set_prefer_keyword(prefer_keyword); input.set_prefer_keyword(prefer_keyword);
if (prefer_keyword) if (prefer_keyword)
input_.set_keyword_mode_entry_method(metrics::OmniboxEventProto::TAB); input.set_keyword_mode_entry_method(metrics::OmniboxEventProto::TAB);
input_.set_from_omnibox_focus(zero_suggest); input.set_from_omnibox_focus(zero_suggest);
OnOmniboxQuery(controller_.get(), input_.text()); OnOmniboxQuery(controller_.get(), input);
controller_->Start(input_); controller_->Start(input_);
} }
......
...@@ -43,7 +43,7 @@ class OmniboxPageHandler : public AutocompleteControllerDelegate, ...@@ -43,7 +43,7 @@ class OmniboxPageHandler : public AutocompleteControllerDelegate,
// OmniboxControllerEmitter::Observer overrides: // OmniboxControllerEmitter::Observer overrides:
void OnOmniboxQuery(AutocompleteController* controller, void OnOmniboxQuery(AutocompleteController* controller,
const base::string16& input_text) override; const AutocompleteInput& input) override;
void OnOmniboxResultChanged(bool default_match_changed, void OnOmniboxResultChanged(bool default_match_changed,
AutocompleteController* controller) override; AutocompleteController* controller) override;
......
...@@ -219,7 +219,7 @@ class AutocompleteChangeObserver : public OmniboxControllerEmitter::Observer { ...@@ -219,7 +219,7 @@ class AutocompleteChangeObserver : public OmniboxControllerEmitter::Observer {
// OmniboxControllerEmitter::Observer: // OmniboxControllerEmitter::Observer:
void OnOmniboxQuery(AutocompleteController* controller, void OnOmniboxQuery(AutocompleteController* controller,
const base::string16& input_text) override {} const AutocompleteInput& input) override {}
void OnOmniboxResultChanged(bool default_match_changed, void OnOmniboxResultChanged(bool default_match_changed,
AutocompleteController* controller) override { AutocompleteController* controller) override {
if (run_loop_.running()) if (run_loop_.running())
......
...@@ -35,7 +35,7 @@ void OmniboxController::StartAutocomplete( ...@@ -35,7 +35,7 @@ void OmniboxController::StartAutocomplete(
if (client_->GetOmniboxControllerEmitter()) { if (client_->GetOmniboxControllerEmitter()) {
client_->GetOmniboxControllerEmitter()->NotifyOmniboxQuery( client_->GetOmniboxControllerEmitter()->NotifyOmniboxQuery(
autocomplete_controller_.get(), input.text()); autocomplete_controller_.get(), input);
} }
// We don't explicitly clear OmniboxPopupModel::manually_selected_match, as // We don't explicitly clear OmniboxPopupModel::manually_selected_match, as
......
...@@ -69,9 +69,9 @@ void OmniboxControllerEmitter::RemoveObserver(Observer* observer) { ...@@ -69,9 +69,9 @@ void OmniboxControllerEmitter::RemoveObserver(Observer* observer) {
void OmniboxControllerEmitter::NotifyOmniboxQuery( void OmniboxControllerEmitter::NotifyOmniboxQuery(
AutocompleteController* controller, AutocompleteController* controller,
const base::string16& input_text) { const AutocompleteInput& input) {
for (Observer& observer : observers_) for (Observer& observer : observers_)
observer.OnOmniboxQuery(controller, input_text); observer.OnOmniboxQuery(controller, input);
} }
void OmniboxControllerEmitter::NotifyOmniboxResultChanged( void OmniboxControllerEmitter::NotifyOmniboxResultChanged(
......
...@@ -23,7 +23,7 @@ class OmniboxControllerEmitter : public KeyedService { ...@@ -23,7 +23,7 @@ class OmniboxControllerEmitter : public KeyedService {
// Invoked when new autocomplete queries are made from the omnibox // Invoked when new autocomplete queries are made from the omnibox
// controller or when those queries' results change. // controller or when those queries' results change.
virtual void OnOmniboxQuery(AutocompleteController* controller, virtual void OnOmniboxQuery(AutocompleteController* controller,
const base::string16& input_text) = 0; const AutocompleteInput& input) = 0;
virtual void OnOmniboxResultChanged(bool default_match_changed, virtual void OnOmniboxResultChanged(bool default_match_changed,
AutocompleteController* controller) = 0; AutocompleteController* controller) = 0;
}; };
...@@ -43,7 +43,7 @@ class OmniboxControllerEmitter : public KeyedService { ...@@ -43,7 +43,7 @@ class OmniboxControllerEmitter : public KeyedService {
// Notifies registered observers when new autocomplete queries are made from // Notifies registered observers when new autocomplete queries are made from
// the omnibox controller or when those queries' results change. // the omnibox controller or when those queries' results change.
void NotifyOmniboxQuery(AutocompleteController* controller, void NotifyOmniboxQuery(AutocompleteController* controller,
const base::string16& input_text); const AutocompleteInput& input);
void NotifyOmniboxResultChanged(bool default_match_changed, void NotifyOmniboxResultChanged(bool default_match_changed,
AutocompleteController* controller); AutocompleteController* controller);
......
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