Bug 925521 - Part 2: correctly record identifiers for non-pre-installed engines. r=gps

* * *
Bug 925521 - Review comments.
This commit is contained in:
Richard Newman 2013-10-18 12:31:39 -07:00
Родитель 80262df43e
Коммит 286bec1974
6 изменённых файлов: 20 добавлений и 46 удалений

Просмотреть файл

@ -3083,8 +3083,7 @@ const BrowserSearch = {
* FHR records only search counts and nothing pertaining to the search itself.
*
* @param engine
* (string) The name of the engine used to perform the search. This
* is typically nsISearchEngine.name.
* (nsISearchEngine) The engine handling the search.
* @param source
* (string) Where the search originated from. See the FHR
* SearchesProvider for allowed values.

Просмотреть файл

@ -318,9 +318,8 @@ BrowserGlue.prototype = {
reporter.onInit().then(function record() {
try {
let name = subject.QueryInterface(Ci.nsISearchEngine).name;
reporter.getProvider("org.mozilla.searches").recordSearch(name,
"urlbar");
let engine = subject.QueryInterface(Ci.nsISearchEngine);
reporter.getProvider("org.mozilla.searches").recordSearch(engine, "urlbar");
} catch (ex) {
Cu.reportError(ex);
}

Просмотреть файл

@ -483,7 +483,7 @@
// null parameter below specifies HTML response for search
var submission = this.currentEngine.getSubmission(aData, null, "searchbar");
BrowserSearch.recordSearchInHealthReport(this.currentEngine.name, "searchbar");
BrowserSearch.recordSearchInHealthReport(this.currentEngine, "searchbar");
openUILinkIn(submission.uri.spec, aWhere, null, submission.postData);
]]></body>
</method>

Просмотреть файл

@ -165,11 +165,12 @@ let AboutHome = {
Cu.reportError(ex);
break;
}
let engine = Services.search.currentEngine;
#ifdef MOZ_SERVICES_HEALTHREPORT
window.BrowserSearch.recordSearchInHealthReport(data.engineName, "abouthome");
window.BrowserSearch.recordSearchInHealthReport(engine, "abouthome");
#endif
// Trigger a search through nsISearchEngine.getSubmission()
let submission = Services.search.currentEngine.getSubmission(data.searchTerms, null, "homepage");
let submission = engine.getSubmission(data.searchTerms, null, "homepage");
window.loadURI(submission.uri.spec, null, submission.postData);
break;
}

Просмотреть файл

@ -1248,7 +1248,6 @@ SearchCountMeasurement2.prototype = Object.freeze({
});
function SearchCountMeasurement3() {
this.nameMappings = null;
SearchCountMeasurementBase.call(this);
}
@ -1261,32 +1260,14 @@ SearchCountMeasurement3.prototype = Object.freeze({
return Services.search.getEngines();
},
_initialize: function () {
this.nameMappings = {};
let engines = this.getEngines();
for (let engine of engines) {
let name = engine.name;
if (!name) {
// This is something we'd like to know, but we can't track it unless we
// rejig how recordSearchInHealthReport is implemented.
continue;
getEngineID: function (engine) {
if (!engine) {
return "other";
}
let id = engine.identifier;
if (!id) {
continue;
if (engine.identifier) {
return engine.identifier;
}
// TODO: again, we need to rejig this to avoid name collisions.
this.nameMappings[name] = id;
}
},
getEngineID: function (engineName) {
if (!this.nameMappings) {
this._initialize();
}
return this.nameMappings[engineName] || "other-" + engineName;
return "other-" + engine.name;
},
});
@ -1320,9 +1301,7 @@ this.SearchesProvider.prototype = Object.freeze({
* Record that a search occurred.
*
* @param engine
* (string) The search engine used. If the search engine is unknown,
* the search will be attributed to "other-$engine"; otherwise, its
* identifier will be used.
* (nsISearchEngine) The search engine used.
* @param source
* (string) Where the search was initiated from. Must be one of the
* SearchCountMeasurement2.SOURCES values.

Просмотреть файл

@ -21,10 +21,6 @@ function MockSearchCountMeasurement() {
}
MockSearchCountMeasurement.prototype = {
__proto__: bsp.SearchCountMeasurement3.prototype,
getEngines: function () {
return DEFAULT_ENGINES;
},
};
function MockSearchesProvider() {
@ -59,16 +55,16 @@ add_task(function test_record() {
if (engine.identifier == "yahoo") {
continue;
}
yield provider.recordSearch(engine.name, "abouthome");
yield provider.recordSearch(engine.name, "contextmenu");
yield provider.recordSearch(engine.name, "searchbar");
yield provider.recordSearch(engine.name, "urlbar");
yield provider.recordSearch(engine, "abouthome");
yield provider.recordSearch(engine, "contextmenu");
yield provider.recordSearch(engine, "searchbar");
yield provider.recordSearch(engine, "urlbar");
}
// Invalid sources should throw.
let errored = false;
try {
yield provider.recordSearch(DEFAULT_ENGINES[0].name, "bad source");
yield provider.recordSearch(DEFAULT_ENGINES[0], "bad source");
} catch (ex) {
errored = true;
} finally {
@ -98,7 +94,7 @@ add_task(function test_record() {
// Also, check that our non-default engine contributed, with a computed
// identifier.
let identifier = "other-Not Default";
let identifier = "notdef";
for (let source of ["abouthome", "contextmenu", "searchbar", "urlbar"]) {
let field = identifier + "." + source;
do_check_true(day.has(field));