Closed Bug 1203161 Opened 4 years ago Closed 4 years ago

remove the browser.search.cache.enabled pref

Categories

(Firefox :: Search, defect, P2)

defect

Tracking

()

RESOLVED FIXED
Firefox 43
Tracking Status
firefox43 --- fixed

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [hijacking][fxsearch])

Attachments

(2 files)

This preference was introduced in bug 394979, but the comments there don't indicate why it was needed. I suspect it was added just so that the cache could be turned off on aurora if regressions were discovered. This pref doesn't seem to have any use case right now, and the cache being optional makes it unsuitable to store additional data about user-installed search engine.
Blocks: 1203167
Attached patch PatchSplinter Review
Assignee: nobody → florian
Attachment #8659265 - Flags: review?(dtownsend)
Comment on attachment 8659265 [details] [diff] [review]
Patch

Over to Drew
Attachment #8659265 - Flags: review?(dtownsend) → review?(adw)
Comment on attachment 8659265 [details] [diff] [review]
Patch

Review of attachment 8659265 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for the -w patch!
Attachment #8659265 - Flags: review?(adw) → review+
Backing out Bug 1184220 wasn't enough to fix the bustage, apparently, so I'm backing out everything else from that push to hopefully get back to a clean slate.
https://hg.mozilla.org/integration/fx-team/rev/dde32e10af8b
https://hg.mozilla.org/mozilla-central/rev/b5297f88b43f
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Looks like this comment:

https://hg.mozilla.org/mozilla-central/rev/b5297f88b43f#l1.11

is now either wrong, or you should not have removed the "if (!this._readOnly)" conditional?
Flags: needinfo?(florian)
I think the comment should be removed.

I haven't seen any data about how often the update feature is used, but I suspect it's very little.

In bug 1203167, I'm going to restrict the update feature to only user-installed engines (as app-shipped engines, and extension-shipped engines both already have a different update mechanism they can use), so engines with updates will never be read-only.

I think we should remove the update feature altogether eventually if my (and Mike's) guess that it isn't actually used in the wild is correct, but I think that'll require adding Telemetry probes to confirm, and so will have to wait a few cycles to have data.
Flags: needinfo?(florian)
(In reply to Florian Quèze [:florian] [:flo] from comment #10)
> I think the comment should be removed.

Yes, I agree. It took me until I was in bed unable to sleep last night for me to realize it :)

> I haven't seen any data about how often the update feature is used, but I
> suspect it's very little.

It has certainly never been used with default engines (i.e. non-profile engines). Bug 511017 and associated bugs never went anywhere.

I suspect it is pretty much never used for profile engines either.

Your plan (disable now for readonly engines, eventually remove entirely) sounds good.
You need to log in before you can comment on or make changes to this bug.