[sqlite] sql::Recovery working under USE_SYSTEM_SQLITE.

Removes the USE_SYSTEM_SQLITE conditional sections in sql::Recover
implementation and tests.  third_party/sqlite includes sqlite_recover
library for the USE_SYSTEM_SQLITE case, with recover.c compiled against
the system version of SQLite.

Additionally remove interiorCursorEOF() from recover.c, as it was
unused, rather than adding a compile flag to allow it.

[Relands https://codereview.chromium.org/1757653002/ ]

BUG=584407
TBR=sdefresne@chromium.org

Review URL: https://codereview.chromium.org/1782633002

Cr-Commit-Position: refs/heads/master@{#380287}

sql/BUILD.gn

@@ -113,6 +113,8 @@ test("sql_unittests") {
   #    '../testing/android/native_test.gyp:native_test_native_code',
   #  ],
   #}],
+
+  # TODO(GYP): dep on copy_test_data_ios action.
 }
 
 if (is_android) {

sql/recovery.cc

@@ -98,11 +98,7 @@ void RecordRecoveryEvent(RecoveryEventType recovery_event) {
 // static
 bool Recovery::FullRecoverySupported() {
   // TODO(shess): See comment in Init().
-#if defined(USE_SYSTEM_SQLITE)
-  return false;
-#else
   return true;
-#endif
 }
 
 // static
@@ -203,27 +199,14 @@ bool Recovery::Init(const base::FilePath& db_path) {
     return false;
   }
 
-  // TODO(shess): Figure out a story for USE_SYSTEM_SQLITE.  The
-  // virtual table implementation relies on SQLite internals for some
-  // types and functions, which could be copied inline to make it
-  // standalone.  Or an alternate implementation could try to read
-  // through errors entirely at the SQLite level.
-  //
-  // For now, defer to the caller.  The setup will succeed, but the
-  // later CREATE VIRTUAL TABLE call will fail, at which point the
-  // caller can fire Unrecoverable().
-#if !defined(USE_SYSTEM_SQLITE)
+  // Enable the recover virtual table for this connection.
   int rc = recoverVtableInit(recover_db_.db_);
   if (rc != SQLITE_OK) {
     RecordRecoveryEvent(RECOVERY_FAILED_VIRTUAL_TABLE_INIT);
     LOG(ERROR) << "Failed to initialize recover module: "
                << recover_db_.GetErrorMessage();
     return false;
   }
-#else
-  // If this is infrequent enough, just wire it to Raze().
-  RecordRecoveryEvent(RECOVERY_FAILED_VIRTUAL_TABLE_SYSTEM_SQLITE);
-#endif
 
   // Turn on |SQLITE_RecoveryMode| for the handle, which allows
   // reading certain broken databases.

sql/recovery_unittest.cc

@@ -172,9 +172,6 @@ TEST_F(SQLRecoveryTest, RecoverBasic) {
             ExecuteWithResults(&db(), kXSql, "|", "\n"));
 }
 
-// The recovery virtual table is only supported for Chromium's SQLite.
-#if !defined(USE_SYSTEM_SQLITE)
-
 // Test operation of the virtual table used by sql::Recovery.
 TEST_F(SQLRecoveryTest, VirtualTable) {
   const char kCreateSql[] = "CREATE TABLE x (t TEXT)";
@@ -737,7 +734,6 @@ TEST_F(SQLRecoveryTest, Bug387868) {
     EXPECT_TRUE(sql::Recovery::Recovered(std::move(recovery)));
   }
 }
-#endif  // !defined(USE_SYSTEM_SQLITE)
 
 // Memory-mapped I/O interacts poorly with I/O errors.  Make sure the recovery
 // database doesn't accidentally enable it.

sql/sql.gyp

@@ -114,6 +114,18 @@
             '../testing/android/native_test.gyp:native_test_native_code',
           ],
         }],
+        ['OS == "ios"', {
+          'actions': [{
+            'action_name': 'copy_test_data',
+            'variables': {
+              'test_data_files': [
+                'test/data',
+              ],
+              'test_data_prefix' : 'sql',
+            },
+            'includes': [ '../build/copy_test_data_ios.gypi' ],
+          }],
+        }],
       ],
       # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
       'msvs_disabled_warnings': [4267, ],

third_party/sqlite/BUILD.gn

@@ -223,13 +223,25 @@ if (use_system_sqlite) {
 
   source_set("sqlite") {
     public_configs = [ ":sqlite_config" ]
+    deps = [
+      ":sqlite_recover",
+    ]
     if (is_ios) {
       deps = [
         ":sqlite_regexp",
       ]
     }
   }
 
+  source_set("sqlite_recover") {
+    sources = [
+      # TODO(shess): Move out of the SQLite source tree, perhaps to ext/.
+      "src/src/recover.c",
+      "src/src/recover.h",
+      "src/src/recover_varint.c",
+    ]
+  }
+
   if (is_ios) {
     source_set("sqlite_regexp") {
       defines = [

third_party/sqlite/sqlite.gyp

@@ -67,6 +67,7 @@
           'conditions': [
             ['OS == "ios"', {
               'dependencies': [
+                'sqlite_recover',
                 'sqlite_regexp',
               ],
               'link_settings': {
@@ -242,6 +243,18 @@
     },],
     ['OS == "ios"', {
       'targets': [
+        {
+          # Virtual table used by sql::Recovery to recover corrupt
+          # databases, for use with USE_SYSTEM_SQLITE.
+          'target_name': 'sqlite_recover',
+          'type': 'static_library',
+          'sources': [
+            # TODO(shess): Move out of the SQLite source tree, perhaps to ext/.
+            'src/src/recover_varint.c',
+            'src/src/recover.c',
+            'src/src/recover.h',
+          ],
+        },
         {
           'target_name': 'sqlite_regexp',
           'type': 'static_library',

third_party/sqlite/src/src/recover.c

@@ -701,8 +701,6 @@ static int getRootPage(sqlite3 *db, const char *zDb, const char *zTable,
  * interiorCursorDestroy - release all resources associated with the
  *                         cursor and any parent cursors.
  * interiorCursorCreate - create a cursor with the given parent and page.
- * interiorCursorEOF - returns true if neither the cursor nor the
- *                     parent cursors can return any more data.
  * interiorCursorNextPage - fetch the next child page from the cursor.
  *
  * Logically, interiorCursorNextPage() returns the next child page
@@ -719,11 +717,6 @@ static int getRootPage(sqlite3 *db, const char *zDb, const char *zTable,
  * Note that while interiorCursorNextPage() will refuse to follow
  * loops, it does not keep track of pages returned for purposes of
  * preventing duplication.
- *
- * Note that interiorCursorEOF() could return false (not at EOF), and
- * interiorCursorNextPage() could still return SQLITE_DONE.  This
- * could happen if there are more cells to iterate in an interior
- * page, but those cells refer to invalid pages.
  */
 typedef struct RecoverInteriorCursor RecoverInteriorCursor;
 struct RecoverInteriorCursor {
@@ -839,14 +832,6 @@ static unsigned interiorCursorChildPage(RecoverInteriorCursor *pCursor){
   return 0;
 }
 
-static int interiorCursorEOF(RecoverInteriorCursor *pCursor){
-  /* Find a parent with remaining children.  EOF if none found. */
-  while( pCursor && pCursor->iChild>=pCursor->nChildren ){
-    pCursor = pCursor->pParent;
-  }
-  return pCursor==NULL;
-}
-
 /* Internal helper.  Used to detect if iPage would cause a loop. */
 static int interiorCursorPageInUse(RecoverInteriorCursor *pCursor,
                                    unsigned iPage){