application-services icon indicating copy to clipboard operation
application-services copied to clipboard

Refactor places DB initialization code

Open bendk opened this issue 3 years ago • 0 comments

The current places DB initialization code is different depending on which connection type we're initializing. This means we often have the following situation:

  1. We create a ReadWrite connection when we create PlacesApi which runs some of the initialization code
  2. Later on down the road, we create a Sync connection, which runs the rest of the initialization code.

However, step 2 happens after startup so we could also have a ReadWrite connection trying to execute a query while the Sync connection is initializing. This was one of the causes of #4762. In general, it seems simpler and safer for all initialization code to run when we create the initial ReadWrite connection.

I had a change to do this, but it caused the fennec and IOS bookmark import tests to fail. Apparently those tests rely on the initialization code to run after the migration. It's possible that the real world migration code also relies on that order.

We could try to investigate this, but maybe the best path forward is to wait until we remove those imports. AFAIK, all the fennec migration code is going away soon (release 97 or 98) and there's an open issue to remove the IOS import code

None of this is pressing because I'm pretty sure the changes in #4782 will fix the immediate issues. But I do think it's worth cleaning up this code if we can.

┆Issue is synchronized with this Jira Task

bendk avatar Jan 18 '22 19:01 bendk