1493483 - In production code, use nsIBrowserSearchService::curentEngine instead of defaultEngine

Status: RESOLVED FIXED

(Firefox :: Search, defect, P3)

Description

[Siddhant [:Siddhant085]]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.0 Safari/605.1.15

Comment 1

[Siddhant [:Siddhant085]]

Was defaultEngine and currentEngine different in the past. I found this code

     // If the search bar is visible, use the current engine, otherwise, fall
     // back to the default engine.
     if (isElementVisible(this.searchBar))
       engine = Services.search.defaultEngine;
     else
       engine = Services.search.defaultEngine;

Comment 2

[Mark Banner (:standard8)]

Yes, they were different at some stage. So we should clean up that code and drop the if statement.

Comment 3

[Siddhant [:Siddhant085]]

Attachment: Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code)

nsIBrowserSearchService::currentEngine and nsIBrowserSearchService::defaultEngine are the same thing.
The use of defaultEngine makes more sense and thus we are phasing out the use of currentEngine and replace it with defaultEngine.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c35bab726b03
Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code) r=Standard8

Comment 5

[Natalia Csoregi [:nataliaCs]]

Backed out for multiple browser-chrome failures e.g browser_extension_controlled.js

backout: https://hg.mozilla.org/integration/autoland/rev/dd418f0ef29cf4082b1379211b601fcf950836da

push with failures https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed,busted,exception,running,pending,runnable&selectedJob=204306963&revision=c35bab726b030312c53f48a654d5fcc2e1264ac2&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=204306963&repo=autoland&lineNumber=2425

[task 2018-10-09T19:44:23.235Z] 19:44:23     INFO - Console message: [JavaScript Error: "TypeError: Services.search.deafultEngine is undefined; can't access its "name" property" {file: "chrome://browser/content/parent/ext-chrome-settings-overrides.js" line: 90}]
[task 2018-10-09T19:44:23.235Z] 19:44:23     INFO - Buffered messages finished
[task 2018-10-09T19:44:23.235Z] 19:44:23     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_extension_controlled.js | Test timed out - 
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_extension_controlled.js | Extension not fully unloaded at test shutdown - 
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - Stack trace:
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - chrome://mochikit/content/tests/SimpleTest/ExtensionTestUtils.js:ExtensionTestUtils.loadExtension/<:112
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - chrome://mochikit/content/browser-test.js:nextTest:695
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - chrome://mochikit/content/browser-test.js:timeoutFn:1190
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - setTimeout handler*chrome://mochikit/content/browser-test.js:Tester_execTest:1152
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - chrome://mochikit/content/browser-test.js:nextTest/<:986
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:795
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - Not taking screenshot here: see the one that was previously logged
[task 2018-10-09T19:44:23.236Z] 19:44:23     INFO - TEST-UNEXPECTED-FAIL | browser/components/preferences/in-content/tests/browser_extension_controlled.js | Extension left running at test shutdown -

Comment 6

[Siddhant [:Siddhant085]]

One problem causing browser_extension_controlled.js to fail is the incorrect spelling of default in ext-chrome-settings-overrides.js. I have fixed that and retried the test on my local system. It is still timing out. I am looking into the problem.

Comment 7

[Siddhant [:Siddhant085]]

The test is timing out because of

INFO Failed to retrieve MOZ_UPLOAD_DIR env var

pushing the changes to run the tests on try server.

Comment 8

[Mark Banner (:standard8)]

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2920d79f0e6be184085c35ea19eec8272deb743e

Siddhant: can you post an updated patch which fixes the bitrot in PlacesSearchAutocompleteProvider.jsm ?

Comment 9

[Siddhant [:Siddhant085]]

Hey Mark,

I went through the treeherder test summary and I see that uriloader/exthandler/tests/mochitest/browser_auto_close_window.js is failing and it also has a bug filed for it (https://bugzilla.mozilla.org/show_bug.cgi?id=1472761). I can't see any errors involving PlacesSearchAutocompleteProvider.jsm.

Comment 10

[Mark Banner (:standard8)]

(In reply to Siddhant [:Siddhant085] from comment #9)
> I went through the treeherder test summary and I see that
> uriloader/exthandler/tests/mochitest/browser_auto_close_window.js is failing
> and it also has a bug filed for it
> (https://bugzilla.mozilla.org/show_bug.cgi?id=1472761).

I believe that failure was actually a side-effect of something that was wrong on the tree at that time.

> I can't see any
> errors involving PlacesSearchAutocompleteProvider.jsm.

Did you pull the latest and rebase it? If so, if you could push again from the latest just to make sure that would be useful.

I'll push it to try one more time just to make sure there isn't an issue with that auto close test.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/408f7a1a2d0f
Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code) r=Standard8

Comment 12

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/408f7a1a2d0f

Comment 13

[[github robot]]

Commits pushed to master at https://github.com/mozilla/activity-stream

https://github.com/mozilla/activity-stream/commit/4b365ad515f7379fbc4bb370f976418009aa518f
Backport Bug 1493483 - Use nsIBrowserSearchService::defaultEngine instead of currentEngine (in production code) r=Standard8

https://github.com/mozilla/activity-stream/commit/73f173931bd55c1e122454be563ed53b769041ff
Bug 1493483 - Fix unit tests to stub .defaultEngine