Closed
Bug 765839
Opened 12 years ago
Closed 12 years ago
Enable OOP test suite
Categories
(Core :: Storage: IndexedDB, defect)
Core
Storage: IndexedDB
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: bent.mozilla, Assigned: bent.mozilla)
Details
Attachments
(1 file)
64.47 KB,
patch
|
khuey
:
review+
|
Details | Diff | Splinter Review |
We need to ensure that all the indexedDB tests work in a child process. This patch fixes all the races that were unearthed by try and should help us identify anything that breaks OOP indexedDB.
Attachment #634114 -
Flags: review?(khuey)
Comment on attachment 634114 [details] [diff] [review]
Patch, v1
Review of attachment 634114 [details] [diff] [review]:
-----------------------------------------------------------------
/me goes to shower
::: build/automationutils.py
@@ +107,5 @@
>
> def isURL(thing):
> """Return True if |thing| looks like a URL."""
> # We want to download URLs like http://... but not Windows paths like c:\...
> + return thing and len(urlparse(thing).scheme) >= 2
Ditch this.
::: dom/indexedDB/IDBTransaction.cpp
@@ +249,5 @@
> if (!IndexedDatabaseManager::IsMainProcess()) {
> NS_ASSERTION(mActorChild, "Must have an actor!");
>
> mActorChild->SendAllRequestsFinished();
> + //PR_Sleep(PR_SecondsToInterval(2));
Remove this.
::: dom/indexedDB/IndexedDatabaseManager.cpp
@@ +601,5 @@
> + // from the list if necessary.
> + if (op->mDatabases.RemoveElement(aDatabase)) {
> + // Now set up the helper if there are no more live databases.
> + NS_ASSERTION(op->mHelper, "How did we get rid of the helper before "
> + "removing the last database?");
op->mHelper || op->mRunnable is what you want here.
@@ +1098,3 @@
> }
>
> + PRUint32 runCount = service && pool && !databases.IsEmpty() ? 2 : 1;
This is pretty awful. At least name this waitCount instead of runCount.
@@ +1135,4 @@
> }
>
> bool
> +IndexedDatabaseManager::IsClearOriginPending(const nsACString& aOrigin)
This should just wrap FindSynchronizedOp(aOrigin, null).
@@ +1438,5 @@
> + // Remove the directory that contains all our databases.
> + nsCOMPtr<nsIFile> directory;
> + nsresult rv =
> + mgr->GetDirectoryForOrigin(mOrigin, getter_AddRefs(directory));
> + NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Fauled to get directory to remove!");
*Failed
::: dom/indexedDB/IndexedDatabaseManager.h
@@ +90,2 @@
> aCallback, aClosure);
> }
These changes can be can reverted.
@@ +220,5 @@
> void OnDatabaseClosed(IDBDatabase* aDatabase);
>
> // Responsible for clearing the database files for a particular origin on the
> // IO thread. Created when nsIIDBIndexedDatabaseManager::ClearDatabasesForURI
> + // is called. Runs four times, twice on the main thread, then once on the IO
Revert this comment.
@@ +269,5 @@
> + }
> + }
> +
> + static void ExclusiveCallback(nsTArray<nsRefPtr<IDBDatabase> >& aDatabases,
> + void* aClosure);
This needs a better name. One that doesn't imply that you already have exclusive access ... how about "InvalidateOpenedDatabasesCallback"?
::: dom/indexedDB/OpenDatabaseHelper.cpp
@@ +1891,5 @@
>
> IndexedDatabaseManager* mgr = IndexedDatabaseManager::Get();
> NS_ASSERTION(mgr, "This should never be null!");
>
> + rv = mgr->AcquireExclusiveAccess(mDatabase, mDatabase->Origin(), helper,
Revert.
@@ +1920,5 @@
>
> IndexedDatabaseManager* mgr = IndexedDatabaseManager::Get();
> NS_ASSERTION(mgr, "This should never be null!");
>
> + rv = mgr->AcquireExclusiveAccess(mDatabase, mDatabase->Origin(), helper,
Revert.
Attachment #634114 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 2•12 years ago
|
||
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #1)
> @@ +90,2 @@
> > aCallback, aClosure);
> > }
>
> These changes can be can reverted.
That caused build failures, I cleaned up our include story a bit and now we have to pass the origin as an arg.
Comment 4•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Updated•12 years ago
|
Assignee: nobody → bent.mozilla
You need to log in
before you can comment on or make changes to this bug.
Description
•