Enable OOP test suite

RESOLVED FIXED in mozilla16

Status

()

Core
DOM: IndexedDB
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ben Turner (not reading bugmail, use the needinfo flag!), Assigned: Ben Turner (not reading bugmail, use the needinfo flag!))

Tracking

unspecified
mozilla16
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Created attachment 634114 [details] [diff] [review]
Patch, v1

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+
https://hg.mozilla.org/integration/mozilla-inbound/rev/30bfeaf1e175
(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

5 years ago
https://hg.mozilla.org/mozilla-central/rev/30bfeaf1e175
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16

Updated

5 years ago
Assignee: nobody → bent.mozilla
You need to log in before you can comment on or make changes to this bug.