1451715 - Rename {get,set,delete}{Window,Global}Value to match {get,set,delete}CustomTabValue

Status: RESOLVED FIXED

(Firefox :: Session Restore, enhancement, P3)

Description

[Dão Gottwald [:dao]]

In bug 1446719 we renamed getTabValue, setTabValue and deleteTabValue. For consistency and clarity we should do the same for: getWindowValue, setWindowValue, deleteWindowValue, getGlobalValue, setGlobalValue, deleteGlobalValue.

Comment 1

[Manish [:manishkk]]

getWindowValue: https://searchfox.org/mozilla-central/search?q=getWindowValue&case=true&regexp=false&path=browser%2F

setWindowValue: https://searchfox.org/mozilla-central/search?q=setWindowValue&case=true&regexp=false&path=browser%2F

deleteWindowValue: https://searchfox.org/mozilla-central/search?q=deleteWindowValue&case=true&regexp=false&path=browser%2F

getGlobalValue: https://searchfox.org/mozilla-central/search?q=getGlobalValue&case=true&regexp=false&path=browser%2F

setGlobalValue: https://searchfox.org/mozilla-central/search?q=setGlobalValue&case=true&regexp=false&path=browser%2F

deleteGlobalValue: https://searchfox.org/mozilla-central/search?q=deleteGlobalValue&case=true&regexp=false&path=browser%2F

I need to changes in these above files. 

but not in these below files.

* browser/components/extensions/schemas/sessions.json
* https://searchfox.org/mozilla-central/rev/003262ae12ce937950ffb8d3b0fa520d1cc38bff/browser/components/extensions/ext-sessions.js#158,165
* browser/components/extensions/test/browser/browser_ext_sessions_window_tab_value.js

Correct?

Comment 2

[Dão Gottwald [:dao]]

Seems correct!

Comment 3

[Manish [:manishkk]]

How can I test this after changes on my system?

Comment 4

[Dão Gottwald [:dao]]

(In reply to Manish Kumar [:manishkk] from comment #3)
> How can I test this after changes on my system?

By running tests:

./mach mochitest browser/components/sessionstore/test/
./mach mochitest browser/components/extensions/test/browser/browser_ext_sessions_window_tab_value.js

Comment 5

[Jason Chapin]

Hi Dão - may I take this one?

Comment 6

[Dão Gottwald [:dao]]

Sure!

Comment 7

[Jason Chapin]

Attachment: bug-1451715.patch

Requested changes made, tests ran without fail. Hope it looks good for you.
Thanks,
Jason

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 9011032 [details] [diff] [review]
bug-1451715.patch

This looks good, except that we only want to change the internal SessionStore API. The extension API shouldn't change for the time being.

Comment 9

[Jason Chapin]

Attachment: bug-1451715.patch

Undid changes to session extension. Please review thanks!

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 9011898 [details] [diff] [review]
bug-1451715.patch

>--- a/browser/components/sessionstore/SessionStore.jsm
>+++ b/browser/components/sessionstore/SessionStore.jsm

>+setCustomGlobalValue(aKey, aStringValue) {
>+    SessionStoreInternal.setCustomGlobalValue(aKey, aStringValue);
>+  },
>+
>+deleteCustomGlobalValue(aKey) {
>+    SessionStoreInternal.deleteCustomGlobalValue(aKey);
>   },

nit: indentation is off at the method headers

Comment 11

[Jason Chapin]

Attachment: bug-1451715.patch

That nit is legit. Patch updated and re–submitted. Thanks!

Comment 12

[Dão Gottwald [:dao]]

Comment on attachment 9012156 [details] [diff] [review]
bug-1451715.patch

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=af1d913388366a4e801e56a848f12798b58fa083

Comment 13

[Dão Gottwald [:dao]]

Comment on attachment 9012156 [details] [diff] [review]
bug-1451715.patch

Looks good, thanks!

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51cd5f6c6fa3
Rename {get,set,delete}{Window,Global}Value to match {get,set,delete}CustomTabValue. r=dao

Comment 15

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/51cd5f6c6fa3