User:Andrew Sutherland/MailNews/GlobalDatabase/Review
bug 450494 is tracking the process of getting gloda onto the trunk.
This page reflects the status of the feedback from the bug as of comment 39. The following comment chains have not been included because, although making for interesting discussion, aren't particularly actionable, but could form the basis of an action-y thing:
Un-Addressed
Not yet/currently being worked, no notable progress, perhaps some discussion.
bienvenu
indexing preference exposure
(bienvenu c15 q7)do we expect this pref to live permanently? If so, it's worth putting in mailnews.js so that it shows up in about:config and has a default.
let branch = prefService.getBranch("mailnews.database.global.indexer");
(asuth c17 a7) I'm not sure. I think Thunderbird will always want the event-driven indexing active, but it sounds like SeaMonkey may not want it on by default? I created the pref as a better alternative to having chrome reach in to turn that on, but without any deep rationale.
(asuth current status (9/18)) Note that this pref fails to stop some listeners from being registered, which is bad. I need to expose the pref in mailnews.js...
MIME Parsing Assertion
(bienvenu c15 q4) I see this assertion when picking the "index everything" menu item:
mime.dll!MimeObject_parse_eof(MimeObject * obj=0x064878a8, int abort_p=0x00000000) Line 302 + 0x25 bytes C++ mime.dll!MimeContainer_parse_eof(MimeObject * object=0x064878a8, int abort_p=0x00000000) Line 129 + 0xe bytes C++
(asuth c17 a4)I'm not familiar with that one. What's the actual assertion text? What type of message was it trying to process?
(bienvenu c20 p6) I'll save it next time I see it - "already parse eof/eom" was one of them, iirc.
(bienvenu c24) Here's the assertion I see - "obj already parsed" is the string, I think.
mime.dll!MimeObject_parse_eof Line 302 + 0x25 bytes mime.dll!MimeContainer_parse_eof Line 129 + 0xe bytes mime.dll!MimeContainer_finalize Line 98 + 0x10 bytes mime.dll!MimeUntypedText_finalize Line 117 + 0xa bytes mime.dll!mime_free Line 323 + 0xe bytes mime.dll!MimeContainer_finalize Line 109 + 0x9 bytes mime.dll!MimeMessage_finalize Line 126 + 0xa bytes mime.dll!mime_free Line 323 + 0xe bytes mime.dll!mime_display_stream_complete Line 989 + 0x9 bytes mime.dll!nsStreamConverter::OnStopRequest Line 1049 + 0xf bytes msgbsutl.dll!nsMsgProtocol::OnStopRequest Line 389 + 0x5a bytes
SMIME Assertions
(bienvenu c15 q5) as well as this one, quite frequently:
mime.dll!nsSMimeVerificationListener::Release() Line 275 + 0x6c bytes pipnss.dll!nsCOMPtr<nsISMimeVerificationListener>::~nsCOMPtr<nsISMimeVerificationListener>() pipnss.dll!nsSMimeVerificationJob::`scalar deleting destructor'() + 0xf pipnss.dll!nsCertVerificationThread::Run() Line 150 + 0x20 bytes C++ pipnss.dll!nsPSMBackgroundThread::nsThreadRunner(void * arg=0x02aa0c80) Line 45
(asuth c17 a5) I don't think I've actually seen this assertion either, but I had noticed that the SMime stuff was getting involved in an undesirable fashion not too long ago. It apparently feeds events to the lock icon on the message pane, and may be involved in the deadlocks I had noticed a-ways back.
(asuth c18) I believe this is the assertion described by bug 379661. So, while the problem is not confined to gloda, I think I still need to investigate/deal with the SMIME interaction.
(bienvenu c19) yes, I probably just saw it because gloda streams all my messages through libmime, the weakest link :-)
(asuth status (9/18)) I need to look into what's going on with the SMIME stuff.
expmess tree row assertions
(bienvenu c15 q3) I'm seeing lots of assertions about the tree row count getting out of sync - those are a sign of not sending the right notifications to the tree control.
(asuth c17 a3) Yes. I think the hg version of expmess improves that, but I'm not sure it's entirely fixed.
(bienvenu c20 p5) I haven't seen the row count assertions since updating expmess.
(asuth current status (9/18)) This is not fixed. I don't know what exactly is up, but it's an exclusively expmess sort of issue. (I haven't actually looked at it for some time, could be a mistaken off-by-one.) I will fix this because it should be easy.
gloda suffixtree confusing comments
(bienvenu c26 q2) Some of this code is commented out - does that make it inconsistent with the comment about 'infinity'?
// However, if this state is a leaf node (end == 'infinity'), then 'end'
// isn't describing an edge at all and we want to avoid accounting for it.
let delta;
/*
if (state.end != this._infinity)
//delta = index - end + 1;
delta = end - (index - state.length);
else */
delta = index - state.length - end + 1;
gloda suffixtree dump statements
(bienvenu c26 q3) There are a lot of dump statements, e.g.,
dump("considering " + this._str.slice(stringStart, stringEnd) + " with pos " +
stringStart + ":" + stringEnd + " with state " +
aState.start + ":" + aState.end +
"(patFirst:" + patternFirst + " patLast: " + patternLast +
" delta: " + aDelta + ")\n");
if (patternFirst >= stringStart) {
if (!(stringStart in aPresence)) {
dump(" adding! (patternFirst: " + patternFirst + ")\n");
aPresence[stringStart] = true;
aResults.push(this._offsetsToItems[mid*3+2]);
}
}
else {
dump(" disregarding because pattern is not contained. (patternLast:" +
patternLast + " aPatLength: " + aPatLength + " stringstart: " +
stringStart + "\n");
}
these should either be turned into logging, or removed, or turned into calls to a function which calls dump based on a global bool, defaulted to false.
conversation unread messages enhancement
(bienvenu c15 q9) Conversations might want have an attribute for the number of unread messages, though it's easily calculated, obviously.
(asuth c17 a8) What specific use-case are you imagining here? We can easily create a getter that performs this tally (by forcing the messages to be loaded and then tallying). If the goal is just to be able to query for conversations with unread messages, the message attribute schema lets us find conversations with at least one unread message (although the query mechanism does not yet expose that). But if we want to query conversations based on their number of un-read, or ordering them, then it definitely makes sense to add the attribute explicitly and take the complexity hit to maintain it. (We're already part-way there with the popularity for contacts and our desire to track first message date/last message date for the conversation, though.)
(bienvenu c20 p7) Nothing specific - as a user, I find the unread count in a thread to be a very interesting bit of information, often, but I'm not sure if that translates into the need to be able to query based on that. And it's a royal pain when that count gets wrong :-)
dmose
gloda.js :: initMyIdentities
(dmose) It would nice if this method were split up into shorter functions for readability/maintainability. If you don't have time to do this now, just add a TODO comment.
(dmose) myEmailAddresses could be declared inside the for loop
(dmose) (Possibly premature) optimization:
for (let iIdentity=0; iIdentity < numIdentities; iIdentity++) {
let msgIdentity = msgAccountManager.allIdentities.GetElementAt(iIdentity)
.QueryInterface(Ci.nsIMsgIdentity);
If you cache msgAccountManager.allIdentities in a local variable before starting the for loop, you'll avoid an XPConnect crossing every iteration.
(asuth) XPConnect turns out to be oddly expensive, so this sounds reasonable and easy to do. I could also change-up to using fixIterator perhaps.
(dmose) please track the "deal with account addition/modification/removal" TODO somewhere
(dmose) "Me" wants to come from a .properties file, I bet.
myContact = GlodaDatastore.createContact(null, null, fullName || "Me",
0, 0);
(asuth) Ack, good point.
Half-Finished Comments
(dmose) (gloda.js) the kAttrDerived doxygen comment is incomplete
Noun Definition
(gloda.js)
(dmose) defineNoun: the doxygen commentary needs a bit of cleanup: I think the nounMeta property names want to be documented semi-separately in their own doxygen block, to give us a place to document the actual defineNoun function signature itself. Also make sure that the property list is complete (eg includes cacheBudget, queryClass, etc). Also, the parameter ordering in the function signature itself seems backwards from an intuitiveness point of view.
(asuth)Yes, the doxygen could use some love and its own block. Noun ID comes after noun meta because it is optional, but it's probably cleaner to just let the caller put 'id' in the meta object in the first place.
(dmose) instead of inventing the term "first class" that newbies to the code will
have to learn, how about calling the nounMeta property "allowsAttrs" or
"allowsArbitraryAttrs"? This seems like it'll be more obvious.
(asuth) Yes, this sounds like a good idea, I will do it.
(dmose) similarly, would "instanceOf" be a clearer property name than "class"?
(asuth) This is also a good idea. I feel a pang of guilt about using something so close to a keyword, but even if we used "instanceof", spidermonkey would apparently be okay with it. I shall do so.
Standard8
test directory server debug output
(Standard8 c13)
- } else {
- dump("Wants directory: "+prop+"\n");
Andrew I don't see why you feel the need to delete this. If you're getting it a lot, then maybe something is requesting a directory more than it should be (or we need to fix it). However I see it as a useful tool because if obtaining the directory is buried in code somewhere, it can be very hard to realise that the directory isn't being obtained because we haven't provided a test for it at xpcshell level.
(asuth c14So, I thought I was the one who added it in a fit of debugging. I realized this was not the case when it showed up in the patch, but I left it in because it seemed like it might be more confusing to someone who didn't understand how nsIDirectoryServiceProvider and family work.
For example, I was experiencing a problem where popstate.dat could not be found. I saw that output, saw the source of the dump, saw that it was throwing an exception, and assumed that I needed to be providing another directory path in the handler. Of course, it turns out that the exception is actually a normal thing to do and allows the built-in logic to just build off the single directory path we do provide. So the debug there led me down the wrong path.
My concern can easily be mitigated by adding a comment to the code with the reinstatement of the debug. Does that sound good?
(Standard8 c16) Yes, maybe a clearer debug statement as well - I think it'd just be good to highlight if we are getting things wrong.
(asuth status 9/18) I will integrate this into the next test patch update or at commit preparations if that doesn't happen.
Being Addressed
Things being addressed. Next action is asuth's.
bienvenu
idle service use
(bienvenu c15 q1) I don't see the gloda code using the idle service to do background indexing on idle - am I missing that? Is that planned, or does it not seem like the right thing to do?
(asuth c17 a1) Correct, the idle service is not currently used. The initial implementation just used a timer interval to schedule things, and it appears to work reasonably well. Which is, no one who has tried gloda thus far has complained about a lack of responsiveness. In practice, the introduction of the body fetching has actually slowed things down even more.
My original plan was to use the idle service to control the parameters of the timer-based indexing. Those variables being: the number of milliseconds between re-scheduling, and the number of messages processed before we stop processing pending another timer interrupt (captured using a token bucket-style implementation). The idea was that when the machine was idle, we could throttle up our indexing.
I think this general idea is the right idea, because we *do* want to index when the user is around, otherwise we lose the ability to keep up with new messages / changed messages. At least, for new/changed messages. When we are just trying to get gloda up to speed, that part is not as important, but we still want to get up to speed as quickly as possible.
There's also some forthcoming interaction with Emre's automatic IMAP offline-fetching stuff that needs to happen. I haven't entirely thought it through, but it seems like, to a large extent, we could just play follow-the-leader on the offline fetching... but there are a large number of cases that need to be dealt with.
Your thoughts/input are greatly appreciated on this one.
(bienvenu c20 p2) Re using the idle service to know when to throttle up the indexing, I think that's a good plan. My brief experience with gloda and expmess is that indexing while I'm doing stuff is pretty painful, so much so that I would really throttle down the amount of stuff that's done when not idle. When new messages arrive, I would put them on a queue of messages to be indexed, and not index them right away. Similarly, if the user selects 100 messages and deletes them, we'd really like to avoid doing all the gloda housekeeping right away (it looks like you have infrastructure in place to do all this, so it's probably just a matter of being less aggressive while not idle). Also (I apologize for not fully groking the code :-) ), can/do you do the body indexing separate from the basic adding of the header to gloda? As you say, streaming the message through libmime is pretty cpu intensive.
I realize there are tradeoffs between having the gloda data up to date, and keeping the UI performant, and there's always the shutdown case. It might be simplest just to remember which folders have pending operations, and sync those on the next run, instead of building full-blown persistance of your job queues.
I'm on Windows, on a rather old box, but I really did feel the pain when new mail came in and they started getting indexed while I was reading and deleting them...
(dmose c22) Seems like chunking up the work into a finer granularity might be another way to address this.
(asuth current status (9/18)) Indexing is being re-done.
make integration
(bienvenu c27 attached patch) I think you also need this patch to get gloda to build if you build from the top - I had to build from mailnews/db otherwise, to get gloda to build. I haven't finished checking that this will fix it, but I suspect it will.
(asuth current status (9/18)) This patch is good and should be folded into my tree patch.
updateFolder usage
(bienvenu c32) One other issue I've run into - gloda seems to update folders on startup, and I run into an assertion because we're in the middle of classifying new mail as junk or not junk, because the ui is getting new mail at the same time. I've hit this for my pop3 inbox, and the assertion is in nsMsgLocalMailFolder::SpamFilterClassifyMessages. This is a potentially bad assertion because if we get out of sync with our classification requests, it can break things like new mail notification and junk mail processing.
There's no particular reason for the indexer to update local folders, since unlike IMAP, it doesn't really do anything interesting as far as the indexer is concerned. I guess you're doing it to make sure the .msf is up to date...which is a bit of an edge case, but I can see why you'd want to do that. But consider the imap case, where update folder will issue a select on the imap server and download new messages; does the indexer want to do that? As you've pointed out before, you'll need to sync with Emre's auto update stuff as well...
You could actually just call GetDatabaseWithReparse for local mail folders. Or I could add a new method to nsIMsgFolder that does what you want for both kinds of folders. UpdateFolder is really meant more for opening folders in the UI, since it kinda assumes that's what's going on (hence the running of the junk mail controls). Or I could treat UpdateFolder with a null msg window as meaning it's not coming from the UI...
(asuth c34) Right, I initially did this for local folders because sometimes in my testing (because of me, generally), the .msf files did not yet exist. I wanted to use GetDatabaseWithReparse, but wanted to use nsIMsgFolder and avoid a downcast. It seemed like a nice freebie that the IMAP state should be up-to-date to boot. (After all, what's the point of a global index if you have to click on things to get the results to be accurate...)
Since Emre's code will be a given, I think I can rely on it to make sure the IMAP is sufficiently up-to-date. That just leaves avoiding dying when the msf doesn't exist yet. If that means an instanceof check and a call to GetDatabaseWithReparse, that works for me.
dmose
memory issues
(dmose) with the new indexer, I've seen RSIZE get extremely large (> 1G) once. Perhaps re-indexing my store would trigger that behavior again.
(asuth) by new indexer, do you mean the sweep-based, event-augmented indexer completed in changeset 13592e09f549 and pushed on the morning of Sunday, Sep 21? If not, then I think we're okay. If so, then it's cause for concern, although I think it would mainly be the garbage collector's fault for being lazy and maybe we can kick it to make it collect more frequently.
(dmose) I did mean the indexer completed in the changeset you referenced, so I suspect we still have some sort of issue.
(asuth status 10/7) I still need to look into this, especially on OS X.
Need Input
asuth needs more info from peoples, grouped by peoples.
bienvenu
dmose
general
* With the new indexer, I'm seeing a moderate number of error messages of the form:
2008-09-21 12:17:29 gloda.fundattr ERROR Message with subject
'Mark Banner' somehow lacks a valid author. Bailing.
where in every case, the subject is purported to be the author name.
Which makes me think the code is getting confused somewhere.
I think this was a transient problem involving IMAP. Are you still seeing this? I do occasionally see things like "Message with subject somehow lacks a valid author", but I think that is a different problem either involving messages with odd MIME things happening or perhaps an event train hiccup.
gloda.js
- in getIdentitiesForFullMailAddresses, are we actually guaranteed that the email addresses aren't relays?
I don't think the methods claim that they do this. I think the goal is that the attribute providers pierce the relay nonsense so it doesn't happen... since we basically depend on the attribute providers to mark relays as such, perhaps we could require them to do so? The result would be that either we don't know something is a relay, or we know it is and you shouldn't really hear about it. Could you clarify what your desired action would be here? (I'm sorry if my delay in getting to this point has made you forget...)
Believed Addressed
asuth believes these points are addressed. Please sign-off on them or move them to agreed addressed or invent a new notation or something. Next action is the person whose heading it is under.
bienvenu
indexer magic constant
(bienvenu c15 q8) might add a comment referring to the NS_ERROR define you're catching here:
catch ( e if e.result == 0xc1f30001) {
// this means that we need to pend on the update.
this._log.debug("Pending on folder load...");
(asuth 9/18) This was addressed with changeset 048686317d14, see the file diff. Note that since use of updateFolder is going away soon, this will shortly be mooted.
(asuth 9/22)So, changeset e997476f295b leaves this using Cr.blah instead, although the change to Components.results was made earlier.
indexer obsolete comment
(bienvenu c15 q6) Is this comment obsolete? You're not using setTimeout, or this._messenger, in indexer.js
// we need this for setTimeout... what to do about this?
this._messenger = Cc["@mozilla.org/messenger;1"].
createInstance(Ci.nsIMessenger);
this._timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
(asuth c17 a6) Yes, it's obsolete and went away sometime yesterday in a clean-up/update pass. _messenger (which oddly inherited the comment) also went away... I think that was detritus from the mime-body-fetching before it entirely migrated to mimemsg.js.
(asuth 9/18) This happened in changeset a5e649db05eb, only that file was affected.
expmess installation/usage
(bienvenu c15 q2) Should I be installing the expmess xpi to see the status of indexing? Is there a newer version of it than 0.0-m1?
(asuth c17 a2) Yes, expmess is the only way to see what is happening. The XPI is very old and probably will break things. I would install the extension directly from hg. (So, check out https://hg.mozilla.org/users/bugmail_asutherland.org/expmess/, then create a file "expmess@mozillamessaging.com" in your profile's extensions directory that provides the path to the checked-out directory.)
expmess LDAP frowny faces
(bienvenu c28 attached patch) I was never seeing anything from expmess when I clicked on a message, other than the frowny face :-( This fixes it for me. I suspect we were getting an ldap directory, which throws an exception when this is called. Andrew, let me know if you want me to just land this in your repo, or if you want to fix it another way...
(asuth c29 approving patch) LDAP, eh? Feel free to land it in the gloda repo.
(Standard8 c30) Yes, not all address book types can support this function at the moment.
(bienvenu c31 noting commit 3ac64fe0567b) finally coerced hg into letting me push
nsMsgSearchDBView::GetType fall-through
(bienvenu c33) patch provided) make seach view return a view type we were generating an error before because the base class returns an error. Gloda & friends wants this not to error out.
(asuth c35 r+ patch Patch looks good; applies and makes the problem go away. Danke.
(Neil c36 sr+) Basically this function is the inverse of the switch in CreateBareDBView, right?
(bienvenu c37 commit marking) yes, it's the same kind of thing...
code formatting: equal spaces
(bienvenu c15 q8) we generally put spaces around the '=', at least in c++
collection.js
let iWrite=0;
for (let iRead=0; iRead < items.length; iRead++) {
datamodel.js:
let values = this[attrDef.boundName];
for (let iValue=0; iValue < values.length; iValue++)
(asuth) Fixed in changeset 0f5e5f6ba559. 1561f5ddffc9 also has a related change to make "for(" become "for (".
utils variable naming
(bienvenu c26 q1)
md5HashString: function gloda_utils_md5hash(aString) {
let converter = Cc["@mozilla.org/intl/scriptableunicodeconverter"].
createInstance(Ci.nsIScriptableUnicodeConverter);
let trash = {};
converter.charset = "UTF-8";
let emailArr = converter.convertToByteArray(aString, trash);
let hasher = Cc['@mozilla.org/security/hash;1'].
"emailArr" seems an odd var name here, if this is a generic utility function
- -)
(asuth) changed to the more generic 'data'.
dmose
shutdown sqlite/storage assertions
(dmose c38) I shut Thunderbird down while it was in the middle of downloading my giant IMAP Trash folder the first time. Saw the following debug output
2008-09-17 19:11:14 gloda.indexer INFO Shutting Down 2008-09-17 19:11:14 gloda.indexer INFO Event-Driven Indexing is now false 2008-09-17 19:11:14 gloda.datastore INFO Closing pending transaction out for shutdown. 2008-09-17 19:11:14 gloda.datastore INFO Closing async connection WARNING: sqlite3_close failed. There are probably outstanding statements!: file /Users/dmose/s/gloda/src/mozilla/storage/src/mozStorageConnection.cpp, line 236 ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "Component returned failure code: 0x8052000e (NS_ERROR_FILE_IS_LOCKED) [mozIStorageConnection.close]" nsresult: "0x8052000e (NS_ERROR_FILE_IS_LOCKED)" location: "JS frame :: file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/modules/datastore.js :: gloda_ds_shutdown :: line 493" data: no] ************************************************************ 2008-09-17 19:11:14 gloda.datastore INFO Closing async connection ************************************************************ * Call to xpconnect wrapped JSObject produced this error: * [Exception... "Component returned failure code: 0xc1f30001 (NS_ERROR_NOT_INITIALIZED) [mozIStorageConnection.close]" nsresult: "0xc1f30001 (NS_ERROR_NOT_INITIALIZED)" location: "JS frame :: file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/MacOS/modules/datastore.js :: gloda_ds_shutdown :: line 493" data: no] ************************************************************
Doing it a second time, I ended up with a few of these as well:
###!!! ASSERTION: Still pending events!: 'mPendingEvents.Count() == 0', file /Users/dmose/s/gloda/src/mozilla/storage/src/mozStorageEvents.cpp, line 462
Both of these were running without the IMAP changes from Emre, FWIW.
(asuth status 9/18) With the indexer changes, I will have all asynchronous statements have result trackers combined with a shutdown listener to ensure that they flush before we allow shutdown to complete. Of course, the fundamental problem you encountered is that indexing currently doesn't care about shutdown and so keeps going up until the rug gets pulled out from under it... which will be addressed.
(asuth 10/7) I did what I said I would did, and it actually works as of this changeset.
Noun IDs
(dmose) does having _nextNounID not be persisted means that ids saved in the database could change across Thunderbird invocations if for some reason the ordering of calls to defineNoun change? If so, that seems fragile...
(asuth) Correct, noun id's are neither stable nor consistent (except for the internal, hard-coded ones). However, noun id's never touch the database at this time, so it's more a question of being confusing/useless for debugging. (Our full knowledge of attributes currently relies on them being registered at the start of every session; we don't persist their noun info. Given our current feature-set, this does not preclude the ability to garbage collect unused attributes.)
The existence of noun id's at this point is effectively an optimization, since their canonical identifier is either their name or the NOUN_ID constant on their data model class's prototype. They only would need to be persisted if/when we start putting them in the database for generic references that do not rely on parameterized attributes. Which is to say, we could create a "generic-reference" attribute whose parameter is the name of the noun that it is a generic-reference too. That neatly avoids having the noun id touch the database and allows easy introspection. I'm not sure that's the right solution to whatever problem would drive such a need, though.
Would you prefer that move to using some centralized noun ID registry or begin persisting the noun ID's to head off our likely future need?
(dmose) I don't think that needs to be done before landing. Please just track this issue somewhere (a bug, wiki page, wherever), and let's deal with it when a concrete need arises.
(asuth) bug 458992 logged.
general
- (do now or track for later): At some point you might want to pull in a current copy of log4moz.js; the enumerateProperties stuff that landed not too long ago looks _really_ useful
datamodel.js
- does the special property want to be a boolean instead of an integer?
no, this is now an enumerated value.
- explain doesn't appear to be called anywhere, so it can presumably go away
It was used by expmess, but I gutted it because it was causing string issues and turned out to be less useful than I originally planned. now fully excised
- seems like it would make sense to provide a boundName setter rather than allowing Gloda.js to reach inside this GlodaAttributeDef
I've moved it to just be an attribute; if we're not going to be read-only and not have any logic, no need for getters and setters. changeset
- getValueFromInstance calls getAttributeInstances, which only exists on GlodaMessage objects, so presumably it will fail when called on any other type of object. Is this intentional? If so, please document...
As of my contact generalization changes getAttributeInstances is now part of a mix-in that GlodaMessage and GlodaContact both get, plus anyone else who wants in.
explattr.js
- it'd be nice to have a brief (1 sentence) comment at the top describing the file and pointing to the more detailed description of explciit addrs
- the comment above the class object definition appears to be wrong
everybody.js
- instead of adding "" to an exception to force it to be a string, either call the .toString method (best) or use the String() conversion operator (ok too)
rendered moot by more extensive formatting
gloda.js
- Why bother having a local let variable in getMessageForHeader?
It probably existed for debugging, but is now gone, thanks.
- use @return in the documentation for getMessageForHeader, please.
- Please figure out how to track the attribute orphaning issue somewhere (spin-off bug, wiki page...)
Created bug 458989.
utils.js
- A brief comment explaining why dateFormat exists and why it lives where it does would be helpful (unless you want to get rid of it now).
I have nuked it and the gloda message view that required it. See changeset.
- I'm a little confused about why md5HashString is converting from UTF-8.
My understanding is that the way Gecko is currently compiled, Spidermonkey strings are always UCS2/UTF16.
Per the doxygen, the implementation is actually from the nsICryptoHash documentation, so I disclaim responsibility for wackiness. However, I will note that nsICryptoHash::update takes an octet array, increasing the complexity.
- The parseMailAddresses comment is ironically, a bit difficult to parse :-). Maybe clarify the sentence a bit or show an example?
- extractBodySnippet doesn't currently do anything or have any callers. if you plan to use it, feel free to leave, but if it's cruft left over from a previous iteration....
Agreed Addressed
bienvenu
dmose
Processing
Andrew is processing this set of comments. Do not touch!
dmose
- be sure to kill the various UI stuff that we don't need before landing (properties, dtd, preferences stuff, etc.)
- if there isn't one already, please file a bug on adding or changing the nsIMsgFolderNotificationService to have a notifyMsgsAdded method so we don't get so many method calls the first time a folder is opened
gloda.js / everybody.js file structuring
(dmose) your gloda_int.js theory in the js comments sounds right; if it's easy to do (it should be, right?) is now the time? If now's not the time, please file a spinoff bug or figure out some other way to ensure that this work gets tracked...
gloda.js spelling typos
* Inspired by reasonable uses of triple-stores, I have tried to leverage * existing model and terminology rather than rolling out own for everything.
s/out/our/
gloda.js -- * I kinda suspect introducing the concept of predicates into the explanation of attributes is likely to make it harder for many people to understand rather than easier. general -- * for all modules, all exported symbols should have associated documentation comments (these could just be pointers) * API points like getters on exported classes need doxygen commentary. datamodel.js -- * should the argument to getValueFromInstance actually be called aNoun? general -- * some interesting debug spew that makes me think gloda might be streaming something on thread where it's not expecting to be streamed: 2008-09-21 19:39:41 gloda.indexer DEBUG 0 candidate messages 2008-09-21 19:39:41 gloda.indexer DEBUG ...creating new message. body length: 1564 2008-09-21 19:39:41 gloda.datastore DEBUG CreateMessage: 8, 123015, 520, 1207609855000000, 47FAA9FF.6020206@redhat.com WARNING: NS_ENSURE_TRUE(mHeaderSink) failed: file /Users/dmose/s/gloda /src/mailnews/mime/src/mimecms.cpp, line 294 ###!!! ASSERTION: nsSMimeVerificationListener not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/dmose /s/gloda/src/mailnews/mime/src/mimecms.cpp, line 275 ###!!! ASSERTION: PipUIContext not thread-safe: '_mOwningThread.GetThread() == PR_GetCurrentThread()', file /Users/dmose/s/gloda/src/mozilla/security/manager /ssl/src/nsNSSComponent.cpp, line 2739 2008-09-21 19:39:41 gloda.indexer DEBUG *** Indexing message: 186254 : FYI 2008-09-21 19:39:41 gloda.indexer DEBUG 1 candidate messages 2008-09-21 19:39:41 gloda.indexer DEBUG candidate folderID: null messageKey: null datamodel.js -- * "allowing the use of attribute-parameter as an attribute" seems unclear on first reading * s/trigger his caching/trigger it's caching/ * getValueFromInstance: in the case where this._singular is set but instances.length > 1, should we assert or throw? If not, how about documenting exactly what's going to be returned? gloda.js -- * given that most of the rest of code-base uses the word "thread", it would probably be good to include in the documentation of NOUN_CONVERSATION a sentence or two about how this is (and/or isn't) different from that * parameterized identity: ** s/constrainted/constrained/ ** do we want to introduce some JS assertions about the constraints here? I think Fx has a concept of such things... ** The intent of the "if (typeof [...]" is not obvious on first reading; a comment here would help, I think. ** shortNames need to come from .properties files, I suspect * Seems like "nounDef" would be clearer and more readable than "nounMeta". It would also parallel attrDef nicely. ** both bindAttribute and defineAttribute seem long enough that it's a little hard to keep all the necessary state in one's head at once. Maybe split them up? * _bindAttribute ** please either add doxygen comments for the params or at least a @seealso pointing to defineAttribute ** Defining functions completely anonymously can make it hard to understand stacks in (e.g.) Venkman; prefer named functions.
To Be Processed
asuth's exciting holding pen for things....
bienvenu
Will gloda land as an extension or in core?
For gloda to not be an extension, the components need to be added to the packages - see http://mxr.mozilla.org/comm-central/search?string=nsLDAPPrefsService.js for an example
I see a bunch of assertions in AsyncExecute::~AsyncDelete while indexing - "still pending events"
xpcom_core.dll!NS_DebugBreak_P(unsigned int aSeverity=0x00000001, const char * aStr=0x038e2e94, const char * aExpr=0x038e2eac, const char * aFile=0x038e2ec8, int aLine=0x000001ce) Line 359 + 0xc bytes C++ strgcmps.dll!AsyncExecute::~AsyncExecute() Line 462 + 0x2b bytes C++ strgcmps.dll!AsyncExecute::`scalar deleting destructor'() + 0xf bytes C++ strgcmps.dll!AsyncExecute::Release() Line 523 + 0x90 bytes C++ xpc3250.dll!XPCJSRuntime::GCCallback(JSContext * cx=0x049f2c60, JSGCStatus status=JSGC_END) Line 818 + 0x14 bytes C++ jsd3250.dll!jsds_GCCallbackProc(JSContext * cx=0x049f2c60, JSGCStatus status=JSGC_END) Line 531 + 0xe bytes C++ gklayout.dll!DOMGCCallback(JSContext * cx=0x049f2c60, JSGCStatus status=JSGC_END) Line 3582 + 0x17 bytes C++ xpc3250.dll!XPCCycleCollectGCCallback(JSContext * cx=0x049f2c60, JSGCStatus status=JSGC_END) Line 459 + 0x17 bytes C++ js3250.dll!js_GC(JSContext * cx=0x049f2c60, JSGCInvocationKind gckind=GC_NORMAL) Line 3734 + 0x9 bytes C++ js3250.dll!JS_GC(JSContext * cx=0x049f2c60) Line 2477 + 0xb bytes C++ xpc3250.dll!nsXPConnect::Collect() Line 530 + 0xa bytes C++ xpcom_core.dll!nsCycleCollector::Collect(unsigned int aTryCollections=0x00000001) Line 2256 + 0x19 bytes C++ xpcom_core.dll!nsCycleCollector_collect() Line 2904 + 0x16 bytes C++ gklayout.dll!nsJSContext::CC() Line 3411 + 0x6 bytes C++ gklayout.dll!nsUserActivityObserver::Observe(nsISupports * aSubject=0x00000000, const char * aTopic=0x023d527c, const unsigned short * aData=0x00000000) Line 265 C++ xpcom_core.dll!nsObserverList::NotifyObservers(nsISupports * aSubject=0x00000000, const char * aTopic=0x023d527c, const unsigned short * someData=0x00000000) Line 129 C++ xpcom_core.dll!nsObserverService::NotifyObservers(nsISupports * aSubject=0x00000000, const char * aTopic=0x023d527c, const unsigned short * someData=0x00000000) Line 184 C++ gklayout.dll!nsUITimerCallback::Notify(nsITimer * aTimer=0x04aebf90) Line 220 C++ xpcom_core.dll!nsTimerImpl::Fire() Line 424 C++ xpcom_core.dll!nsTimerEvent::Run() Line 514 C++ xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0012f864) Line 511 C++ xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00d022e8, int mayWait=0x00000001) Line 227 + 0x16 bytes C++ gkwidget.dll!nsBaseAppShell::Run() Line 170 + 0xc bytes C++
I managed to get the UI locked up one time, again while indexing was going on.
dmose
everything below here is done in the ugly-but-easy style!
dmose puts his new comments here...
datamodel.js
--
* having raw attributes defined as a tuple of [attribute id, parameter,
value] means that consumers end up accessing by integer index, which
feels error prone (and makes calling code unnecessarily hard to
read). How about making them be objects instead, so
that the accessors are property names?
e.g. { id: 1, parameter: "foo", value: "bar" }
* while all the getter magic in the MixIn is elegant, it feels to me
like it also makes the code less approachable. This would seem like
another reason to move towards raw storage fields.
/**
* @TODO Return the collection of messages belonging to this conversation.
* (And weakly store a reference to the collection. Once the user is rid of
* it, we really don't care.)
*/
* What's still TODO in the above comment? Also, the last sentence
isn't very clear, please be more explicit about what's intended.
* Should GlodaMessage.toString() use something from a properties file?
Or will this never be displayed to end users?
* GlodaMessage.{_nuke,_ghost} don't appear to be private methods, so
presumably they want to have doxygen comments and to not begin with
an underscore.
* In the GlodaMessage folderMessage getter, shouldn't both of those
LOGs be at a higher level than just "info"?
* Presumably the GlodaContact.pictureURL attribute doesn't want to
have its format and source hardcoded. OK by me to track this
somehow and address later, though.
collection.js
--
* doxygen comment on cacheLookupOneByUniqueValue appears to have been
copied from cacheLookupOne but not modified. When fixing, please be
sure to mention that the caller is expected to guarantee the
"uniqueness" invariant. Backing up a step, though, is there any
particular reason not to collapse these two functions into one by
parameterizing the array to search? (If it turns out that makes it
hard to read, I can live with keeping it as is).
general
--
Note that the next several errors were seen in a build that was
somewhat suspect:
Saw a few of these while indexing:
###!!! ASSERTION: Still pending events!: 'mPendingEvents.Count() == 0', file
/Users/dmose/s/gloda/src/mozilla/storage/src/mozStorageEvents.cpp,
line 462
* Saw this in my error console today:
2008-10-09 14:40:46 gloda.indexer ERROR Inconsistency in
conversations invariant on undefined. It has conv id 3039 but expected 3042
* When running the indexer on my entire mailstore with lightning
installed, I see a bunch of these fly by, presumably for invites that
I've got lying around. Not sure whether this is a lightning or a
calendar issue:
Error: convertToHTML: Cannot create itipItem: [Exception... "Component
returned failure code: 0x80004003 (NS_ERROR_INVALID_POINTER)
[nsIMsgMailNewsUrl.msgWindow]" nsresult: "0x80004003
(NS_ERROR_INVALID_POINTER)" location: "JS frame ::
file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/
Contents/MacOS/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/
components/lightningTextCalendarConverter.js :: lmcCTH :: line 226" data: no]
Source File:
file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/
Contents/MacOS/extensions/%7Be2fda1a4-762b-4020-b5ad-a41df1933103%7D/
components/lightningTextCalendarConverter.js
Line: 236
* GlodaFolder stuff in gloda.js and datamodel.js needs doxygen comment
love
* getFolderForFolder is not a very intuitive name. How about
getFolderForMsgFolder?
collection.js
--
* it would be good for cacheLoadUnify* to have @return and @param dox
> // should we also maintain a unique value mapping...
> if (this._nounMeta.usesUniqueValue)
> this._uniqueValueMap = {};
>
> this.items = aItems || [];
> this._idMap = {};
> if (this._uniqueValueMap) {
If you bump those two initializers up above the first if, you should
be able to collapse the two ifs into one.
* Just saw the following output:
2008-10-10 12:05:05 gloda.indexer DEBUG folderDeleted notification
************************************************************
* Call to xpconnect wrapped JSObject produced this error: *
[Exception... "'[JavaScript Error: "this._datastore is undefined" {file:
"file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/
Contents/MacOS/modules/indexer.js" line: 1591}]' when calling method:
[nsIMsgFolderListener::folderDeleted]" nsresult: "0x80570021
(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "<unknown>"
data: yes]
************************************************************
collection.js
--
* Please add a comment explaining the reasoning behind GlodaCollection
maintaining both an array (items) and an object/hash (_idMap) that
point to the same data.
* It looks like the GlodaCollection constructor duplicates a bunch of code from
_onItemsAdded. Could it just call that method directly?
> _onItemsAdded: function gloda_coll_onItemsAdded(aItems) {
> this.items.push.apply(this.items, aItems);
Why use apply here rather than calling this.items.push() directly?
general
--
In the cases where you don't actually care about the index in a
an Iterator:
foreach (let [iItem, item] in Iterator(aItems))
it's clearer to explicitly ignore the index like so:
foreach let [,item] in Iterator(aItems)
Alternately, you could use items.forEach(), possibly in combination
with an expression closure:
items.forEach(function foo() expression);
collection.js
--
* does _onItemsRemoved want to be _onItemsDeleted for symmetry?
* _onItemsRemoved seems pretty complex for something so basic. It's
not clear to me why making it simpler would be O(n^2). Can you
elaborate?
* the call to this._collectionsByNoun[nounID].filter in
registerCollection would be more readable if it used an
expression closure.
indexer.js
--
> if (msgHdr && !(msgHder.flags&MSG_FLAG_EXPUNGED))
* The lack of spaces around the & operator goes against prevailing style
and makes it hard to read
collection.js
--
* the last half of GlodaLRUCacheCollection.delete appears to be
duplicated in GlodaLRUCacheCollection.add. Factor that out into a
separate method that can be shared? An _unlinkItem could be similarly
factored out of a couple of methods for improved readability. Neither
of these things are huge wins, though, so if you want to do them later
in another bug, that's fine.
> // (_lruNext cannot be null)
* I suspect that making the above comment a bit more explicit about
why it's true will help prevent future code-maintainers from
inadvertently breaking this invariant.
gloda.js
--
* Do the constrainer functions in _bindAttribute really want to live
in query.js? Perhaps the thing to do is to the solve the
_bindAttribute naming issue by splitting _bindAttribute itself
into _bindAttribute (which could stay in gloda.js) and
_setAttributeConstrainers (which could live in query.js). If it's
helpful, a _setupAttribute convenience method that calls both could be
written.
* As suggested elsewhere, please make sure all your functions have actual names
so that they are meaningful in stacks.
* As far as I can tell, usesParameter in nounMeta is not set by any
callers. Should it be discarded, or should (at least)
the parameterized-identity definition set it?
* in _bindAttribute, it would be clearer if nounMeta were explicitly
called objNounMeta (or objNounDef, as suggested elsewhere), to
contrast it with subjectNounMeta. Also, the comment referring to
"on-object" bindings is confusing because it's referring
to a JS object representing the Gloda "subject" in the middle of
code that touches Gloda "objects".
datamodel.js
--
* Given that GlodaHasAttributeMixin.get_rawAttributes uses
getMessageAttributes under the hood, why/how does it actually work
when mixed into an (eg) GlodaContact?
query.js
--
* public GlodaQueryClass methods need doxygen love
Recent debug build lossage #1:
2008-10-12 17:54:20 gloda.indexer DEBUG ...creating new message. body
length: 9553
2008-10-12 17:54:20 gloda.datastore DEBUG CreateMessage: 15, 1688, 19345,
1220919529000000, p0624080ac4eb7152ce3e@[10.0.1.4]
2008-10-12 17:54:20 gloda.indexer DEBUG *** Indexing message: 1689 :
Changemakers + Mozilla
2008-10-12 17:54:21 gloda.indexer DEBUG 0 candidate messages
2008-10-12 17:54:21 gloda.indexer DEBUG ...creating new message. body
length: 1885
2008-10-12 17:54:21 gloda.datastore DEBUG CreateMessage: 15, 1689, 18313,
1220907407000000, C4EADF9F.E419%cdahle@ashoka.org
2008-10-12 17:54:21 gloda.indexer DEBUG *** Indexing message: 1690 :
Changemakers + Mozilla
2008-10-12 17:54:21 gloda.indexer DEBUG 1 candidate messages
2008-10-12 17:54:21 gloda.indexer DEBUG candidate folderID: null
messageKey: null
2008-10-12 17:55:01 gloda.datastore ERROR Synchronous step gave up after
32000 tries.
2008-10-12 17:55:04 gloda.datastore ERROR Synchronous step gave up after
32000 tries.
2008-10-12 17:55:06 gloda.datastore ERROR Synchronous step gave up after
32000 tries.
2008-10-12 17:55:09 gloda.datastore ERROR Synchronous step gave up after
32000 tries.
************************************************************
* Call to xpconnect wrapped JSObject produced this error: *
[Exception... "'[JavaScript Error: "authorIdentity.contact is null" {file:
"file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents
/MacOS/modules/fundattr.js" line: 384}]' when calling method:
[mozIStorageStatementCallback::handleCompletion]" nsresult: "0x80570021
(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "<unknown>" data:
yes]
************************************************************
2008-10-12 17:55:14 gloda.indexer DEBUG Detected un-idle, throttling down.
###!!! ASSERTION: Still pending events!: 'mPendingEvents.Count() ==
0', file/Users/dmose/s/gloda/src/mozilla/storage/src/mozStorageEvents.cpp,
line 462
Recent debug build lossage #2:
2008-10-12 17:00:41 gloda.autocomp DEBUG initializing completers
2008-10-12 17:00:41 gloda.datastore DEBUG QUERY FROM QUERY:
SELECT * FROM contacts WHERE id IN (SELECT id FROM contacts
WHERE ((popularity >= 10)) )
2008-10-12 17:00:43 gloda.datastore DEBUG QUERY FROM QUERY:
SELECT * FROM identities WHERE id IN (SELECT id FROM identities WHERE
((contactID = 1 OR contactID = 2 OR contactID = 3 OR contactID = 4 OR
contactID = 5 OR contactID = 6 OR contactID = 7 OR contactID = 8 OR
contactID = 9 OR contactID = 10 OR contactID = 60 OR contactID = 92 OR
contactID = 96 OR contactID = 228 OR contactID = 298 OR contactID =
299 OR contactID = 303 OR contactID = 307 OR contactID = 311 OR
contactID = 312 OR contactID = 313 OR contactID = 314 OR contactID =
315 OR contactID = 316 OR contactID = 317 OR contactID = 319 OR
contactID = 320 OR contactID = 346 OR contactID = 347 OR contactID =
348 OR
[...]
= 9307 OR contactID = 9308 OR contactID = 9309 OR contactID = 9310 OR
contactID = 9312 OR contactID = 9313 OR contactID = 9314 OR contactID
= 9315 OR contactID = 9316 OR contactID = 9317 OR contactID = 9318 OR
contactID = 9319 OR contactID = 9320 OR contactID = 9321 OR contactID
= 9322 OR contactID = 9323 OR contactID = 9324 OR contactID = 9325 OR
contactID = 9326 OR contactID = 9330 OR contactID = 9342 OR contactID
= 9343 OR contactID = 9344 OR contactID = 9345 OR contactID = 9348 OR
contactID = 1)) ) - 1: Expression tree is too large (maximum depth
1000) - [Exception... "Component returned failure code: 0x80004005
(NS_ERROR_FAILURE) [mozIStorageConnection.createStatement]" nsresult:
"0x80004005 (NS_ERROR_FAILURE)" location: "JS frame ::
file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/
Contents/MacOS/modules/datastore.js
:: gloda_ds_createSyncStatement :: line 712" data: no]' when calling
method: [nsIFactory::createInstance]" nsresult: "0x8057001e
(NS_ERROR_XPC_JS_THREW_STRING)" location: "JS frame ::
chrome://experimentaltoolbar/content/overlay.js :: anonymous :: line
97" data: no]
************************************************************
************************************************************
* Call to xpconnect wrapped JSObject produced this error: *
[Exception... "Component returned failure code: 0x80570016
(NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]" nsresult:
"0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)" location: "JS frame ::
chrome://experimentaltoolbar/content/overlay.js :: anonymous :: line
97" data: no]
************************************************************
query.js
--
* The use of "explicit" in naming GlodaExplicitQueryClass is a little
non-obvious to newbies; it'd be nice if we could think of something
better. So far, I haven't been able to, however. :-/
* The purity-of-essence part of me kinda wonders if there shouldn't be
a separate array for all these constrainer functions, but maybe it's
not worth it.
* Looks like neither WILDCARD nor or()/unions/owner are actually used by any
callers. Unless you know of a specific use case for them, I'm
somewhat inclined to suggest axing them in the interest of
simplicity. This would also allow the newQuery documentation
comment in gloda.js to be made more understandable as well. On the
other hand, this isn't a huge win.
* I think test() would be easier to grok if you split the then and
else clauses of |if attribDef.singular| out into separate functions.
It might be worth collapsing them into one since they're almost identical,
but it's possible that the split is better for readability.
* The whole structure of the APV as "sometimes [attr, param, value]"
and "sometimes that stuff + other things in a contextually
determined ordering" seems both hard to read and error-prone. This
wants to be a hash, I suspect.
glautocomp.js
--
* in ACGR_addRows, why use apply? It appears to be setting |this| to
what it would have been set to anyway.
* nsAutoCompleteGlodaResult._problem doesn't appear to ever actually
be set anywhere. Can it be removed?
* Given that the nsAutoCompleteGlodaResult constructor sets
this._results to [] by default, it looks to me as though the test
for null in the matchCount getter can never be true, so it's
probably not necessary.
* the references to "rich" in the comments a bit non-obvious; maybe be
a bit more explicit?
* the various uses of thing.{somegetter} mostly appear to be
abstraction violations. :-)
general
--
* I pulled the latest GloDa / expmess / exptoolbar earlier in the week, patched
mozStorage to use only one thread, and then saw a bunch of errors, which I'll
paste below:
2008-10-24 20:08:44 gloda.everybody INFO ... loading resource://gloda
/modules/fundattr.js
2008-10-24 20:08:44 gloda.everybody ERROR !!! error loading
resource://gloda/modules/fundattr.js
2008-10-24 20:08:44 gloda.everybody ERROR (undefined:61)
[Exception... "Component returned failure code: 0x80520012
(NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]" nsresult:
"0x80520012 (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame ::
file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/
MacOS/modules/everybody.js
:: loadModule :: line 61" data: no]
2008-10-24 20:08:44 gloda.everybody INFO ... loading resource://gloda
/modules/noun_mimetype.js
2008-10-24 20:08:44 gloda.everybody ERROR !!! error loading
resource://gloda/modules/noun_mimetype.js
2008-10-24 20:08:44 gloda.everybody ERROR (undefined:61)
[Exception... "Component returned failure code: 0x80520012
(NS_ERROR_FILE_NOT_FOUND) [nsIXPCComponents_Utils.import]" nsresult:
"0x80520012 (NS_ERROR_FILE_NOT_FOUND)" location: "JS frame ::
file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/
MacOS/modules/everybody.js
:: loadModule :: line 61" data: no]
2008-10-24 20:08:51 gloda.ds.qfq DEBUG handleCompletion: contact
2008-10-24 20:08:51 gloda.ds.qfq DEBUG defer: -1 resolved: 1
2008-10-24 20:08:51 gloda.ds.qfq DEBUG QFQR: about to trigger
listener: [object Object]with collection: contact
************************************************************
* Call to xpconnect wrapped JSObject produced this error: *
[Exception... "'[JavaScript Error: "this.identityCollection is undefined"
{file: "file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app
/Contents/
MacOS/components/glautocomp.js" line: 265}]' when calling method:
[mozIStorageStatementCallback::handleCompletion]" nsresult: "0x80570021
(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "<unknown>"
data: yes]
************************************************************
2008-10-24 22:21:33 gloda.indexer DEBUG <<< _indexMessage
2008-10-24 22:21:33 gloda.indexer DEBUG *** Indexing message: 204553 :
2008-10-24 22:21:33 gloda.indexer WARN Bailing on job (at
undefined:148) because: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIMsgMessageService.streamMessage]" nsresult:
"0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: file:///Users/dmose
/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/
MacOS/modules/mimemsg.js :: MsgHdrToMimeMessage :: line 148" data: no]
2008-10-24 22:21:33 gloda.indexer INFO --- Done indexing,
disabling timer renewal.
Type name in exptoolbar, hit return:
************************************************************
* Call to xpconnect wrapped JSObject produced this error: *
[Exception... "'[JavaScript Error: "constraints is not defined" {file:
"file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/
MacOS/modules/gloda.js" line: 1079}]' when calling method:
[nsIAutoCompleteSearch::startSearch]" nsresult: "0x80570021
(NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS)" location: "<unknown>"
data: yes]
************************************************************
2008-10-25 00:29:47 expmess.overlay INFO Exception at
file:///Users/dmose/s/gloda/obj-tb/mozilla/dist/ShredderDebug.app/Contents/
MacOS/modules/gloda.js:236:TypeError:
query.folder is not a function
Since your most-recent landing in mozilla-central, I've dropped the one-thread
patch and pulled the latest of all three repos again this afternoon (Thurs,
10/29). I've since seen another occurence of the identityCollection error, as
well as a number of instances of the "bailing on job" error.