Better error checking and lazy statements for bookmarks service

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Toolkit
Places
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: mak, Assigned: mak)

Tracking

Trunk
mozilla1.9.3a1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Ts])

Attachments

(5 attachments, 1 obsolete attachment)

(Assignee)

Description

9 years ago
Get better error checking and kick old code.
(Assignee)

Comment 1

9 years ago
Created attachment 421301 [details] [diff] [review]
Part1: better error checking and introduce lazy statements code.

I'll do in 2 parts, this is big enough though (and sadly). In next part i'll convert old statements to use the lazy statements code.
Attachment #421301 - Flags: review?(dietrich)
(Assignee)

Comment 2

9 years ago
oh there is a failure in test_placesTxn, looks like we try to set an empty keyword, i'll check that.
(Assignee)

Updated

9 years ago
Attachment #421301 - Attachment is obsolete: true
Attachment #421301 - Flags: review?(dietrich)
(Assignee)

Comment 3

9 years ago
Comment on attachment 421301 [details] [diff] [review]
Part1: better error checking and introduce lazy statements code.

my fault, new patch coming.
(Assignee)

Comment 4

9 years ago
Created attachment 421356 [details] [diff] [review]
Part1: better error checking and introduce lazy statements code.
Attachment #421356 - Flags: review?(dietrich)
(Assignee)

Comment 5

9 years ago
Created attachment 421435 [details] [diff] [review]
Part2: Convert current global statements to lazy statements

This one converts 1:1 the old statements

I'll probably provide a third part, i'd like to make a new macro
GET_AND_ASSIGN_LAZY_STATEMENT(stmt, mDBStmt); that will

mozIStorageStatement* stmt = GetStatement(mDBStmt);
NS_ENSURE_STATE(stmt);
mozIStorageScoper scoper(stmt);

since this code is repeated in various services now.
and probably make the local macro in GetStatement be a global macro in Helpers.h
Attachment #421435 - Flags: review?(dietrich)
(Assignee)

Comment 6

9 years ago
Also, while touching this i've noticed how many statements we have in bookmarks service, sounds a bit crazy seeing them, what are we doing so complex?!
(Assignee)

Comment 7

9 years ago
Created attachment 421459 [details] [diff] [review]
Part3: consolidate lazy statements boilerplate in macros
Attachment #421459 - Flags: review?(dietrich)
(Assignee)

Comment 8

9 years ago
Created attachment 421463 [details] [diff] [review]
Part4: rename ENUMERATE_OBSERVERS to NOTIFY_OBSERVERS

you asked me this renaming some time ago, let's do it.
Attachment #421463 - Flags: review?(dietrich)
(Assignee)

Updated

9 years ago
Attachment #421463 - Attachment description: rename ENUMERATE_OBSERVERS to NOTIFY_OBSERVERS → Part4: rename ENUMERATE_OBSERVERS to NOTIFY_OBSERVERS
(Assignee)

Comment 9

9 years ago
Created attachment 421469 [details] [diff] [review]
Part5: kill mDBGetItemIndex

this statement can be easily replaced
Attachment #421469 - Flags: review?(dietrich)
Comment on attachment 421356 [details] [diff] [review]
Part1: better error checking and introduce lazy statements code.

whew, that's a lot of cleanup. r=me!
Attachment #421356 - Flags: review?(dietrich) → review+
Attachment #421435 - Flags: review?(dietrich) → review+
Attachment #421459 - Flags: review?(dietrich) → review+
Attachment #421463 - Flags: review?(dietrich) → review+
Attachment #421469 - Flags: review?(dietrich) → review+
(Assignee)

Comment 12

9 years ago
Backed out due to unexplainable failures in Linux and Max Xd boxes... they fail on RemoveItem, a negative or 0 id is passed to it... no idea what's up, vut looks like the same issue causing some orange...

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263647218.1263651393.26473.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263643733.1263646383.31164.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263645378.1263648688.26329.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1263646914.1263649426.3072.gz
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 13

9 years ago
Windows Xpcshell and X0 machines were passing.
(Assignee)

Comment 14

9 years ago
bah, sounds like was just memory corruption due to stupid casts. Sounds crazy that tryserver runs did not notice anything. Btw, i'll push again first 2 parts once i can build in Linux debug. Mac is happy afaict.
(Assignee)

Comment 15

9 years ago
and Linux is happy too, here.
(Assignee)

Comment 16

9 years ago
pushed again the first 2 parts, if they'll stick, i'll push the other ones, since the issue was related to part2 afaict.
http://hg.mozilla.org/mozilla-central/rev/afcae9cfb798
http://hg.mozilla.org/mozilla-central/rev/7a7d0da67610
(Assignee)

Comment 17

9 years ago
yeah, was that, pushed remaining parts.
http://hg.mozilla.org/mozilla-central/rev/6c91fcc70f18
http://hg.mozilla.org/mozilla-central/rev/5e098d789ca0
http://hg.mozilla.org/mozilla-central/rev/4b8bf926f4e6
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.