User-Agent is incorrectly cached in settings.json, with stale value remembered on upgrades

[andersk]

We seem to be saving and restoring (part of) the User-Agent in settings.json, leading to self-contradictory user-agents like

ZulipElectron/5.0.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_3) AppleWebKit/537.36 (KHTML, like Gecko) Zulip/4.0.3 Chrome/66.0.3359.181 Electron/3.1.10 Safari/537.36

(Note the ZulipElectron/5.0.0 and Zulip/4.0.3.)

This setting seems to have been added in #818. There should be no reason to persist the User-Agent to disk at all. Why are we doing that?

Cc @kanishk98

[akashnimare]

There should be no reason to persist the User-Agent to disk at all. Why are we doing that?

The main reason I think is when the app is freshly installed there is no user-agent and we need to have this while adding the servers from the setting page, #817 should contain more info. But I think it's worth revisiting the implementation of this.

[andersk]

I don’t understand. There “is no User-Agent”, so the solution is…to store the User-Agent to disk?

[timabbott]

Yeah, that doesn't make any sense to me. The parts of the User-Agent above that aren't this bit from disk come from the browser.

Yes, we do need to set User-Agent header when making that HTTP request, but what does that have to do with what's stored on disk?

[abhigyank]

I think it makes sense to use session.fromPartition('webview:persistsession').getUserAgent() every time userAgent is required. We could store this to the disk at startup everytime and read from disk when it is required during that run-time in renderer process.
Currently the value for this is - Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Zulip/5.0.0 Chrome/80.0.3987.158 Electron/8.2.0 Safari/537.36

[andersk]

There is no reason to store the User-Agent to disk at all, ever. I don’t understand what problem you think that is solving.

[abhigyank]

There is ideally no reason to do it. Since session is a main process object, I suppose it would be better to access it in the beginning of the app run and store it which can be read from the renderer process. Otherwise we would need to read the UserAgent of session in renderer process using remote. Both ways we ensure correct User Agent.

[andersk]

If we’re trying to avoid remote (#915), the right alternative is ipcRenderer. If we’re trying to avoid doing the IPC multiple times, store it in a variable in memory. There’s still no reason to store it to disk.

[abhigyank]

I am not sure but I don't suppose we can access main process variables from renderer. Is storing on the disk a bad approach?

[andersk]

Yes. Storing it to disk is a bad approach.

Cons:

• The disk is much less efficient than memory—up to hundreds of thousands of times slower.
• Repeatedly writing to a solid-state disk wears it out slowly over time.
• Without properly synchronizing the disk accesses (using IPC!), we might read stale values.

Pros:

• None.
• Absolutely none at all.
• Why was this even an option that was considered?

The renderer process has variables too.

[akashnimare]

If we can get the User-agent from the "session" in the renderer process we don't need to save anything.

[abhigyank]

Yeah, so we can do one thing to avoid remote and multiple ipc calls -
In dom-load in main process, send the value of userAgent to renderer process, and received that in system-util and set the value of userAgent to a variable in it which can be resued as and when required. So this would be a variable in memory of renderer. I think should work @andersk .

[akashnimare]

@manavmehta can you open a WIP PR with your fix?