Changes between Initial Version and Version 1 of Ticket #12683, comment 5


Ignore:
Timestamp:
Jul 29, 2014, 12:34:29 AM (5 years ago)
Author:
isis
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #12683, comment 5

    initial v1  
    1010|| extensions.torbutton.block_disk || true ||
    1111|| extensions.torbutton.saved.disk_cache || true ||
     12 
     13So it seems like, perhaps, if TorButton's `block_disk` patches are disabled, then FF writes the `nsIPermissionsManager` preferences to disk, and TorButton isn't able to clear them from disk, even when calling `Ci.nsIPermissionManager.removeAll()`. Perhaps `Ci.nsIPermissionManager.removeAll()` is broken?
     14 
     15Additionally, and I believe this supports my theory that `nsIPermissionManager.removeAll()` is broken, TorButton sets
    1216
    13 So it seems like, perhaps, if TorButton's `block_disk` patches are disabled, then FF writes the `nsIPermissionsManager` preferences to disk, and TorButton isn't able to clear them from disk, even when calling `nsIPermissionsManager.removeAll()`. Perhaps `nsIPermissionsManager.removeAll()` is broken?
     17|| '''Preference Name''' || '''Value''' ||
     18|| permissions.memory_only || true ||
    1419
     20when
     21
     22|| '''Preference Name''' || '''Value''' ||
     23|| extensions.torbutton.block_disk || true ||
     24
     25and likewise `false` when the other is set to `false`.
     26 
     27There is another method, `RemoveAllInternal` in the `nsPermissionManager` [https://mxr.mozilla.org/mozilla-esr24/source/extensions/cookie/nsPermissionManager.cpp#899 implementation]:
     28
     29{{{
     30899 nsresult
     31900 nsPermissionManager::RemoveAllInternal(bool aNotifyObservers)
     32901 {
     33902   // Remove from memory and notify immediately. Since the in-memory
     34903   // database is authoritative, we do not need confirmation from the
     35904   // on-disk database to notify observers.
     36905   RemoveAllFromMemory();
     37906   if (aNotifyObservers) {
     38907     NotifyObservers(nullptr, NS_LITERAL_STRING("cleared").get());
     39908   }
     40909
     41910   // clear the db
     42911   if (mDBConn) {
     43912     nsCOMPtr<mozIStorageAsyncStatement> removeStmt;
     44913     nsresult rv = mDBConn->
     45914       CreateAsyncStatement(NS_LITERAL_CSTRING(
     46915          "DELETE FROM moz_hosts"
     47916       ), getter_AddRefs(removeStmt));
     48917     MOZ_ASSERT(NS_SUCCEEDED(rv));
     49918     if (!removeStmt) {
     50919       return NS_ERROR_UNEXPECTED;
     51920     }
     52921     nsCOMPtr<mozIStoragePendingStatement> pending;
     53922     mozIStorageStatementCallback* cb = new DeleteFromMozHostListener(this);
     54923     rv = removeStmt->ExecuteAsync(cb, getter_AddRefs(pending));
     55924     MOZ_ASSERT(NS_SUCCEEDED(rv));
     56925
     57926     return rv;
     58927   }
     59928
     60929   return NS_OK;
     61930 }
     62}}}
     63
     64which is called from the `nsPermissionManager::RemoveAll` [https://mxr.mozilla.org/mozilla-esr24/source/extensions/cookie/nsPermissionManager.cpp#876 method] of the `nsIPermissionManager` [https://mxr.mozilla.org/mozilla-esr24/source/netwerk/base/public/nsIPermissionManager.idl#130 interface]. Earlier in the same implementation, there are [https://mxr.mozilla.org/mozilla-esr24/source/extensions/cookie/nsPermissionManager.cpp#207 some notes in the docs] mentioning a reference cycle:
     65 
     66{{{
     67211  * Note: Beware that, if you hold onto a |CloseDatabaseListener| from a
     68212  * |nsPermissionManager|, this will create a cycle.
     69}}}
     70
     71So, my best understanding so far of this bug is that TorButton sets `permissions.memory_only=false` if `extensions.torbutton.block_disk=false`, which seems to cause `nsPermissionManager` to try to remove the preferences from the database, but `nsPermissionManager::RemoveAllInternal` doesn't raise any sort of error if it can't scrub and re-initialise the database, and so it only successfully runs the `RemoveAllFromMemory()` method, leaving the databases as they were when the preferences were written to disk.
     72
     73Perhaps because the database access is async, when `nsPermissionManager::RemoveAll` is called, it calls `nsPermissionManager::RemoveAllFromMemory`, which sets the pointer for the `nsPermissionManager` to `nullptr`, and then calls `nsPermissionManager::RemoveAllInternal` which tries to handle updating the database asynchronously, and in the meantime TorButton is closing all the windows (a.k.a. "closing the browser") which might trigger the [https://mxr.mozilla.org/mozilla-esr24/source/extensions/cookie/nsPermissionManager.cpp#217 CloseDatabaseListener] to fire, causing the database writes to stop, leaving the changes erased from memory and not from disk when `"New Identity"` is clicked.
     74