Page MenuHomePhabricator
Differential D11670

[native][web] Don't stamp SQLite database until authoritative keyserver auth
AbandonedPublic
Actions

Authored by ashoat on Tue, Apr 16, 12:47 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 9:07 PM
Unknown Object (File)
Sat, Apr 27, 10:41 AM
Unknown Object (File)
Fri, Apr 26, 2:23 PM
Unknown Object (File)
Thu, Apr 25, 1:07 PM
Unknown Object (File)
Sun, Apr 21, 10:41 AM
Unknown Object (File)
Wed, Apr 17, 3:38 AM
Unknown Object (File)
Wed, Apr 17, 3:37 AM
Unknown Object (File)
Wed, Apr 17, 3:36 AM
Subscribers
None

Details

Reviewers
tomek
inka
Summary

This addresses ENG-7708.

Depends on D11668

Test Plan

I walked through the scenario that triggered the "Bad Credentials" bug (ENG-7450) and confirmed that it no longer occurred. Scenario is described here

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Tue, Apr 16, 12:47 PM
ashoat mentioned this in D11534: [web][native] Extract the common logic determining when to clear the data.Tue, Apr 16, 12:49 PM
ashoat added inline comments.Tue, Apr 16, 12:52 PM
native/data/sqlite-data-handler.js
150

The new changes will make this code clear the SQLite database when EITHER the authoritative keyserver cookie OR the currentUserInfo is unset. Theoretically that means the SQLite database might be cleared sooner or more often than before, but in practice I think it will be the same as before. The code in KeyserverConnectionHandler should make sure that we trigger a log out if the authoritative keyserver cookie is invalidated. Note that this should not prevent keyserver session recovery, since we defer invalidating the authoritative keyserver cookie in Redux until the recover fails

155

In D11534, @tomek updates the equivalent condition for web, but doesn't touch this one. It feels weird that they're inconsistent. Should I update this to match what we're doing on web?

Harbormaster completed remote builds in B28251: Diff 39162.Tue, Apr 16, 1:03 PM
ashoat requested review of this revision.Tue, Apr 16, 1:03 PM
inka added inline comments.Wed, Apr 17, 6:01 AM
native/data/sqlite-data-handler.js
141–143

can we use isLoggedInToKeyserver?

tomek accepted this revision.Wed, Apr 17, 6:28 AM
tomek added inline comments.
native/data/sqlite-data-handler.js
155

Yes, I think we should keep them consistent.

This revision is now accepted and ready to land.Wed, Apr 17, 6:28 AM
ashoat abandoned this revision.Wed, Apr 17, 7:48 PM

More context in the task, but I'm abandoning this approach in favor of another, more idiomatic/clean one

native/data/sqlite-data-handler.js
155

I'll put up a separate diff for this

ashoat added inline comments.Thu, Apr 18, 12:59 PM
native/data/sqlite-data-handler.js
155

https://phab.comm.dev/D11694

Revision Contents
Changeset List

PathSize
native/
data/
19 lines
web/
shared-worker/
17 lines

Diff 39162

View Options

native/data/sqlite-data-handler.js

Loading...
View Options

web/shared-worker/sqlite-data-handler.js

Loading...