1629939 - Search bar calls into SearchService.defaultEngine twice for each call to updateDisplay

Status: RESOLVED FIXED

(Firefox :: Search, defect, P3)

Description

[Mark Banner (:standard8)]

Currently the search bar's updateDisplay() function is doing two private browsing checks and calling into SearchService.defaultEngine (or defaultPrivateEngine) twice each time it is called:

https://searchfox.org/mozilla-central/rev/2cd2d511c0d94a34fb7fa3b746f54170ee759e35/browser/components/search/content/searchbar.js#230,234

Although we cache the search engine defaults, this seems highly unnecessary and is causing us to go across the component boundaries twice when we don't need to.

We should therefore simplify that function to use an intermediate variable.

This won't have a significant effect on performance but it would help with debugging and reduce unnecessary calls to the search service.

Comment 1

[Mark Banner (:standard8)]

To help Mozilla out with this bug, here's the steps:

1. Comment here on the bug that you want to volunteer to help. I (or someone else) will assign it to you.
2. Download and build the Firefox source code, see the [docs for details](https://firefox-source-docs.mozilla.org/contributing/how_to_contribute_firefox.html. An artifact build is all you need.
   • If you have any problems, please ask on Matrix in the #introduction channel. They're there to help you get started.
3. Start working on this bug - fix the issue described in comment 0 and test the changes.

• For testing, running the tests in the browser/components/search directory should suffice. Use ./mach mochitest browser/components/search to run those.

4. Create a commit with a message like "Bug 1629939 - Use an intermediate variable in MozSearchbar.updateDisplay to avoid calling into the search service twice. r?Standard8"
5. Once the patch is submitted, I'll take a look. If there's any changes necessary I'll comment in Phabricator, so be prepared to update the patches.
   • If you do need to update the commit, please amend the existing commit, rather than creating new ones. This helps with tracking of review comments.
6. Once we're happy with the changes, I'll push it to autoland - our integration branch. Your code will then soon be shipping to Firefox users worldwide!
7. Now you get to think about what kind of bug you'd like to work on next. Let me know what you're interested in and I can help you find your next contribution.

Comment 2

[Ved Dandekar]

Hey!
Could I please be assigned this issue?

Comment 3

[Mark Banner (:standard8)]

Hi Ved, sure.

Comment 4

[Ved Dandekar]

Attachment: Bug 1629939 - Remove obsolete code that is setting the iconURI in MozSearchbar.updateDisplay. r?Standard8

Depends on D71003

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/68d0f6719da5
Remove obsolete code that is setting the iconURI in MozSearchbar.updateDisplay. r=Standard8

Comment 6

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/68d0f6719da5