Note: There are a few cases of duplicates in user autocompletion which are being worked on.
Bug 553413 (async-IndexedDB)

Implement Asynchronous parts of the Indexed Database API

RESOLVED FIXED in mozilla2.0

Status

()

Core
DOM: Core & HTML
P1
normal
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: sdwilsh, Assigned: sdwilsh)

Tracking

({dev-doc-complete})

Trunk
mozilla2.0
dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta1+)

Details

(Whiteboard: [evang-wanted])

Attachments

(3 attachments, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Updated

7 years ago
Alias: async-IndexedDB
Whiteboard: [evang-wanted]
(Assignee)

Comment 1

7 years ago
So, what I'm going to do here is spin off sub bugs to do the chunks of work, and every so often I'll land them to mozilla-central when it makes sense to do so (such as "we have opening and creating object stores done, let's land).
(Assignee)

Updated

7 years ago
Depends on: 554693
(Assignee)

Updated

7 years ago
Depends on: 555316

Updated

7 years ago
blocking2.0: ? → beta1+
(Assignee)

Updated

7 years ago
No longer depends on: 554693
(Assignee)

Updated

7 years ago
No longer depends on: 555316
(Assignee)

Comment 2

7 years ago
Created attachment 450528 [details] [diff] [review]
v0.1

This is still rough, and I'll be going over it tomorrow more to see if I have things that I see need to be fixed (and some stuff needs to be pulled out into its own bug).

Getting it up so people can start looking it over though...

Generated with the following command:
diff -x .hg -x idb.diff -x build -x config -r -U8 -p -N ../clea
n-mozilla-central/ . > idb.diff
Attachment #450528 - Flags: review?(jst)
(Assignee)

Comment 3

7 years ago
Created attachment 450537 [details] [diff] [review]
v0.1.1

Last one had two files marked as changed that shouldn't have been.  This fixes that.
Attachment #450528 - Attachment is obsolete: true
Attachment #450537 - Flags: review?(jst)
Attachment #450528 - Flags: review?(jst)
(Assignee)

Comment 4

7 years ago
jst: please don't review the stuff in storage/.  I'll be spinning that out into a new bug today.
(Assignee)

Updated

7 years ago
Depends on: 571581
(Assignee)

Updated

7 years ago
Depends on: 571599
Comment on attachment 450537 [details] [diff] [review]
v0.1.1

This was one *huge* patch! I did make it through the whole patch (but skipped the storage changes) but only found a couple of problems. So apart from what's listed below this looks good to me!

- In dom/base/nsDOMClassInfo.cpp:

+  NS_DEFINE_CLASSINFO_DATA(IDBRequest, nsEventTargetSH,
+                           DOM_DEFAULT_SCRIPTABLE_FLAGS)

XXX: Looking at nsEventTargetSH it seems to be specifically written to deal with nsXHREventTarget, is that really what we want here?

- In IDBCursorRequest::Update():

+      JSString* str = JSVAL_TO_STRING(prop.value());
+      size_t len = JS_GetStringLength(str);
+      if (!len) {
+        return NS_ERROR_INVALID_ARG;
+      }
+      const PRUnichar* chars =
+        reinterpret_cast<const PRUnichar*>(JS_GetStringChars(str));
+      if (key.StringValue() != nsDependentString(chars, len)) {

Instead typing out all of that you could use nsDependentJSString. Same in a few other place.

- In UpdateHelper::DoDatabaseWork():

+  // TODO update indexes if needed

Bug on file?

- In GetSuccessEvent::GetResult() in IDBEvents.cpp:

+  if (!mJSRuntime) {
+    JSContext* cx;
+    rv = cc->GetJSContext(&cx);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    JSAutoRequest ar(cx);
+
+    JSRuntime* rt = JS_GetRuntime(cx);
+
+    JSBool ok = JS_AddNamedRootRT(rt, &mCachedValue,
+                                  "GetSuccessEvent::mCachedValue");
+    NS_ENSURE_TRUE(ok, NS_ERROR_FAILURE);
+
+    nsCOMPtr<nsIJSON> json(new nsJSON());
+    rv = json->DecodeToJSVal(mValue, cx, &mCachedValue);
+    NS_ENSURE_SUCCESS(rv, rv);
+
+    mJSRuntime = rt;

GetSuccessEvent::~GetSuccessEvent() only calls JS_RemoveRootRT() if mJSRuntime is set, so if the above JSON call fails we'll leak and/or crash later on.

r=jst with that looked into.
Attachment #450537 - Flags: review?(jst) → review+
Keywords: dev-doc-needed
(Assignee)

Comment 6

7 years ago
Created attachment 452305 [details] [diff] [review]
v0.1.1 to v0.2 interdiff

Builds on v0.1.1 by implementing more things, addressing review requests, and adding more tests.
Attachment #452305 - Flags: review?(jst)
(Assignee)

Updated

7 years ago
Depends on: 568898
(Assignee)

Updated

7 years ago
No longer depends on: 568898
Comment on attachment 452305 [details] [diff] [review]
v0.1.1 to v0.2 interdiff

- In dom/indexedDB/IDBObjectStoreRequest.cpp, GetKeyFromObject():

-    size_t len = JS_GetStringLength(str);
-    if (!len) {
-      return NS_ERROR_INVALID_ARG;
-    }
-    const PRUnichar* chars =
-      reinterpret_cast<const PRUnichar*>(JS_GetStringChars(str));
-    aKey = nsDependentString(chars, len);
+    aKey = nsDependentJSString(key);
     return NS_OK;

The !len check got dropped here, is that intentional?

r=jst with that looked into.
Attachment #452305 - Flags: review?(jst) → review+
(In reply to comment #7)
> The !len check got dropped here, is that intentional?

Yeah, it's ok.
Created attachment 452853 [details] [diff] [review]
v0.2 to v0.3 interdiff

Spec complete as of last friday.
Attachment #452853 - Flags: review?(jst)
Comment on attachment 452853 [details] [diff] [review]
v0.2 to v0.3 interdiff

Looks good!
Attachment #452853 - Flags: review?(jst) → review+

Updated

7 years ago
Depends on: 574232

Updated

7 years ago
Depends on: 574257
This is marked as a b1 blocker, and all the dependencies are complete - should this get checked in, marked as fixed ... just trying to determine status.
http://hg.mozilla.org/mozilla-central/rev/e910fd948e5b

This landed yesterday afternoon, sorry.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.3 → mozilla2.0
Documentation:

https://developer.mozilla.org/en/IndexedDB
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.