Closed
Bug 1186805
Opened 9 years ago
Closed 9 years ago
Replace nsBaseHashtable::EnumerateRead() calls in dom/datastore/ with iterators
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: baku)
References
Details
Attachments
(1 file, 1 obsolete file)
10.12 KB,
patch
|
Details | Diff | Splinter Review |
Because iterators are so much nicer than enumerate functions. There are five occurrences of EnumerateRead() in this directory. A note to the assignee: to preserve existing behaviour, you should probably use nsBaseHashtable::Iterator::UserData() rather than nsBaseHashtable::Iterator::Data(). (The latter should be used when replacing nsBaseHashtable::Enumerate()).
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8667321 -
Flags: review?(n.nethercote)
Reporter | ||
Comment 2•9 years ago
|
||
Comment on attachment 8667321 [details] [diff] [review] ds.patch Review of attachment 8667321 [details] [diff] [review]: ----------------------------------------------------------------- Such an improvement. Thank you. r=me with the nits below addressed. ::: dom/datastore/DataStoreService.cpp @@ -331,5 @@ > - > - auto* data = static_cast<GetDataStoreInfosData*>(aUserData); > - if (aAppId == data->mAppId) { > - return PL_DHASH_NEXT; > - } This if-block didn't survive the transition. Is that intended? I can't see why you would remove it. @@ +1023,5 @@ > + continue; > + } > + > + if (!aOwner.IsEmpty() && > + !aOwner.Equals(iter.UserData()->mManifestURL)) { There are five occurrences of iter.UserData() in this function. Please create a local variable for it. @@ +1229,5 @@ > + for (auto iter = apps->ConstIter(); !iter.Done(); iter.Next()) { > + bool readOnly = aReadOnly || iter.UserData()->mReadOnly; > + > + rv = ResetPermission(iter.Key(), iter.UserData()->mOriginURL, > + iter.UserData()->mManifestURL, Again, please factor out the repeated iter.UserData() calls.
Attachment #8667321 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8667321 -
Attachment is obsolete: true
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → amarchesini
Keywords: checkin-needed
Assignee | ||
Comment 4•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4468b86a6203
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4468b86a6203
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•