1779094 - Improve Internal handling of user search settings within SearchSettings.jsm

Status: RESOLVED FIXED

(Firefox :: Search, task, P2)

Description

[Mandy Cheang [:mcheang][she/her]]

Attachment: Screen Shot 2022-07-11 at 5.32.58 PM.png

I'm working on search management improvements for this current sprint.
One of these improvements involves our internal handling of user settings.

We would like to cache the user settings to maintain data integrity. After that is done, it can help with fixing Bug 1596398 so we do not rewrite the user settings on startup every time.

After some thought, I've broken down the task into smaller steps:

• Deep clone _metaData and #currentSettings - Clone version is read only (#cachedMetaData and #cachedCurrentSettings)
• Compare #cachedCurrentSettings and #currentSettings before writing to disk within _write. If it has changed, write to disk, otherwise do not write to disk.
• Create getters and setters for #currentSettings

this._settings.setUserSettingsAttribute

• Create getters and settings for #currentSettings.engines._metadata

this._settings.setEngineMetaData

• Rename setAttribute to setMetaDataAttribute
• Change all calls to setAttribute to setMetaDataAttribute

this._settings.setMetadataAttribute

• Differentiate between userSettingsFile and SettingsObject better by renaming these two objects
• Write tests to compare performance on startup when we're not writing the settings file versus when we are
• Write tests to validate the caching is working
• Write test for new methods added

There's an attachment of the SearchSettings.jsm Object and the userSettings Object that we read from disk to reference the structure of it.

Comment 1

[Mandy Cheang [:mcheang][she/her]]

Attachment: Bug 1779094 - Improve internal handling of user search settings Part II. r=Standard8

Standard8

Comment 2

[Mandy Cheang [:mcheang][she/her]]

Attachment: Bug 1779094 - Improve internal handling of user search settings Part I. r=Standard8

This patches removes the top level settings metadata object.

Any updates to the metadata will be done on the 2nd level object
#currentSettings.metaData instead. Removing the top level metadata object, we
improve code clarity by only having one metadata object and not two
metadata objects.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/32bd53c3dd84
Improve internal handling of user search settings Part I. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/54f9c347df9a
Improve internal handling of user search settings Part II. r=Standard8

Comment 4

[Cristian Tuns]

Backed out for causing talos damp failures

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | damp | toolbox/browser-toolbox.js: Test timed out

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/54110b0b3d1c
Improve internal handling of user search settings Part I. r=Standard8
https://hg.mozilla.org/integration/autoland/rev/fad2e0fe5101
Improve internal handling of user search settings Part II. r=Standard8

Comment 6

[Marian-Vasile Laza]

Re land because we think the problem it's from another push - Bug 1783160

Comment 7

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/54110b0b3d1c
https://hg.mozilla.org/mozilla-central/rev/fad2e0fe5101