1384207 - AllTables includes attached metadata tables

Status: RESOLVED WONTFIX

(Firefox for iOS :: Data Storage, enhancement, P1)

Description

[Richard Newman [:rnewman]]

BrowserTable.AllTables lists all the tables that are meant to be in the DB.

AllIndices and AllViews are similar.

It's used for two purposes:

1. In BrowserTable.drop.
2. In BrowserTable.exists, which is used to check whether the DB is complete.

At the point in which we run (2), the metadata DB _isn't yet attached_. That means our update check always fails, and we move on to an update:

2017-07-25 10:32:24.188 [Debug] [BrowserDB.swift:549] tablesExist > 15 tables exist. Expected 17
2017-07-25 10:32:24.188 [Debug] [BrowserDB.swift:135] updateTable(_:table:) > Trying update BROWSER version 26
2017-07-25 10:32:24.189 [Debug] [BrowserDB.swift:209] createOrUpdate > Table BROWSER already exists.

This is all kinds of backwards. We should not be managing attached tables in BrowserTable, or we should be ensuring that attached databases are attached _immediately_ after opening, before doing anything with the schema.

Consequences of this:

- Logspam and excess work.
- General difficulty in figuring out what state the system will be in.
- Potentially, we will do the check in the log above, and _not update the metadata DB schema_. That could be bad.

Comment 1

[Richard Newman [:rnewman]]

btw, this is on a clean first run.

Comment 2

[Justin D'Arcangelo [:justindarc]]

Attachment: GitHub Pull Request

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 8889978 [details] [review]
GitHub Pull Request

r+ with manual testing.

Comment 4

[Justin D'Arcangelo [:justindarc]]

Marking this as WONTFIX since we'll be getting rid of the attached metadata.db in Bug 1384278.