Closed
Bug 835767
Opened 11 years ago
Closed 11 years ago
AsyncConnectionHelper::ConvertCloneReadInfosToArray callers don't need to clear structured clone buffers
Categories
(Core :: Storage: IndexedDB, defect)
Core
Storage: IndexedDB
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: vendo, Assigned: vendo)
Details
Attachments
(1 file, 2 obsolete files)
|
3.92 KB,
patch
|
bent.mozilla
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:18.0) Gecko/20100101 Firefox/18.0 Build ID: 20130116073211 Expected results: Body of ConvertCloneReadInfosToArray with buffers cleaning is defined here: http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/AsyncConnectionHelper.cpp#594 Cleaning shoud by done after conversion and only once here. http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/IDBIndex.cpp#1545 http://mxr.mozilla.org/mozilla-central/source/dom/indexedDB/IDBObjectStore.cpp#3965
|
||
Updated•11 years ago
|
Assignee: nobody → swenruzicka
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 707560 [details] [diff] [review] bug-835767-fix Review of attachment 707560 [details] [diff] [review]: ----------------------------------------------------------------- Hi Vendelin, thanks for submitting this patch! I think we can make this a little better by simply removing the |mCloneReadInfos[index].mCloneBuffer.clear();| lines from IDBIndex.cpp and IDBObjectStore.cpp. That way we minimize our code duplication. Does that make sense?
Attachment #707560 -
Flags: review?(bent.mozilla)
|
Assignee | |
Comment 3•11 years ago
|
||
Hi, yes it absolutely does. Newer version of patch added.
Attachment #707560 -
Attachment is obsolete: true
Attachment #709109 -
Flags: review?(bent.mozilla)
|
|
||
Comment 4•11 years ago
|
||
The assertion condition should be reversed, I'll do it before landing.
Comment on attachment 709109 [details] [diff] [review] bug-835767-fix v.2 This looks much better, thanks! However there are some unrelated whitespace changes that will confuse the log info for those lines (the "blame") that I'd like for you to remove before we check this in.
Attachment #709109 -
Flags: review?(bent.mozilla)
|
|
||
Updated•11 years ago
|
Summary: AsyncConnectionHelper::ConvertCloneReadInfosToArray shouldn't clear structured clone buffers → AsyncConnectionHelper::ConvertCloneReadInfosToArray callers don't need to clear structured clone buffers
|
|
Assignee | |
Comment 6•11 years ago
|
||
Thanks for reviews :) assertion reversed and whitespace changes removed
Attachment #709109 -
Attachment is obsolete: true
Attachment #709187 -
Flags: review?(bent.mozilla)
Comment on attachment 709187 [details] [diff] [review] bug-835767-fix v.3 Thanks! This looks great.
Attachment #709187 -
Flags: review?(bent.mozilla) → review+
|
||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b2ed3a48edbe
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•