Closed Bug 1024360 Opened 11 years ago Closed 11 years ago

Remove deprecated mozIStorageBaseStatement methods usage from the tree

Categories

(Core :: SQLite and Embedded Database Bindings, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla33

People

(Reporter: mak, Assigned: michael, Mentored)

References

Details

(Whiteboard: [good next bug][lang=js])

Attachments

(1 file, 2 obsolete files)

Some code in the tree is still using the deprecated mozIStorageBaseStatement methods: bindUTF8StringParameter bindStringParameter bindDoubleParameter bindInt32Parameter bindInt64Parameter bindNullParameter bindBlobParameter http://mxr.mozilla.org/mozilla-central/search?string=bind%23Parameter\(&regexp=1
Attached patch Patch (obsolete) — Splinter Review
This patch replaces usage of the deprecated methods on mozIStorageBaseStatement with mozIStorageBindingParams.bindByIndex(). Here are the results from the try server: https://tbpl.mozilla.org/?tree=Try&rev=7b25bad55486
Attachment #8440491 - Flags: review?(mak77)
Mentor: mak77
Whiteboard: [good next bug][mentor=mak][lang=js] → [good next bug][lang=js]
Comment on attachment 8440491 [details] [diff] [review] Patch Review of attachment 8440491 [details] [diff] [review]: ----------------------------------------------------------------- Hi, thanks for your contribution. It looks good, but there are a couple things to figure: 1. You didn't replace the usage at http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/OpenDatabaseHelper.cpp#374 it should use BindXXXByName instead of these methods 1. on Try I see permanent failures on Windows. Off-hand they look unrelated, but still, would be safer to get a second try run (with also the cpp change) and verify that
Attachment #8440491 - Flags: review?(mak77) → feedback+
Attached patch Patch (obsolete) — Splinter Review
Thanks, Marco. I've updated the patch to replace the usage of BindXXXParameter() with BindXXXByIndex() in OpenDatabaseHelper.cpp. And here are the results from the try server for this updated patch: https://tbpl.mozilla.org/?tree=Try&rev=31e9437ccb9d
Attachment #8440491 - Attachment is obsolete: true
Attachment #8446581 - Flags: review?(mak77)
Comment on attachment 8446581 [details] [diff] [review] Patch Review of attachment 8446581 [details] [diff] [review]: ----------------------------------------------------------------- please add r=mak to checkin message ::: dom/indexedDB/OpenDatabaseHelper.cpp @@ +378,3 @@ > NS_ENSURE_SUCCESS(rv, rv); > > + rv = stmt->BindInt64ByIndex(2, dataVersion); I'd prefer BindXXXByName in OpenDatabaseHelper.cpp, since the params are named in the query. No need for a further Try, provided you can compile this code locally.
Attachment #8446581 - Flags: review?(mak77) → review+
Attached patch PatchSplinter Review
I have changed BindXXXParameter() to BindXXXByName() rather than BindXXXByIndex() in OpenDatabaseHelper.cpp as you suggested.
Attachment #8446581 - Attachment is obsolete: true
Attachment #8447457 - Flags: review?(mak77)
Comment on attachment 8447457 [details] [diff] [review] Patch Review of attachment 8447457 [details] [diff] [review]: ----------------------------------------------------------------- It looks good, thank you!
Attachment #8447457 - Flags: review?(mak77) → review+
Attachment #8447457 - Flags: checkin?
Assignee: nobody → michael
Keywords: checkin-needed
Attachment #8447457 - Flags: checkin?
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Product: Toolkit → Core
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: