Closed Bug 450777 Opened 12 years ago Closed 12 years ago
Node Info Manager::Get Node Info
patch coming soon
I'd like to land one of these patches. I've cleaned up the generated code a bit. Do you have any more suggestions as to what needs to be better(other than indentation) before this can land?
Assignee: nobody → tglek
Status: NEW → ASSIGNED
Attachment #334028 - Flags: review?(jst)
Would be landing with this as part of the bigger patch
sorry, forgot to qrefresh
Attachment #334031 - Attachment is obsolete: true
Comment on attachment 334028 [details] [diff] [review] automagical patch + rv = nodeInfo ? NS_OK : NS_ERROR_FAILURE; NS_ENSURE_SUCCESS(rv, rv); It'd be nice if this could be changed to NS_ENSURE_TRUE(nodeInfo, NS_ERROR_FAILURE), and extra bonus points for automatically fixing up indentation too! :) r+sr=jst!
Jim, I need your emacs powers!
another automatic rev
Attachment #334028 - Attachment is obsolete: true
I'm frustrated with the outparamdelled getter nsNodeInfoManager::GetNodeInfo() as I don't know how to make xpcom not leak in that case.
(In reply to comment #7) > Created an attachment (id=338207) [details] > full patch(auto + manual) > > I'm frustrated with the outparamdelled getter nsNodeInfoManager::GetNodeInfo() > as I don't know how to make xpcom not leak in that case. I think we'll inevitably need to make this return an already_AddRefed<nsINodeInfo> and have the places that use raw pointers with this return value use .get() on the return, i.e. nsINodeInfo *foo = nsNodeInfoManager::GetNodeInfo(...).get(), or something along those lines.
so the logic here if someone called the getter with getter_AddRefs(), then it becomes foo = getter() in other cases the form is foo = getter().get() I think this gets correct behavior for cases like nsNodeInfoManager::GetTextNodeInfo().
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 338370 [details] [diff] [review] alreadyAddRefed patch >+ NS_ENSURE_TRUE(newNodeInfo, NS_ERROR_FAILURE); > NS_ASSERTION(newNodeInfo, "GetNodeInfo lies"); This makes no sense...
Depends on: 534162
You need to log in before you can comment on or make changes to this bug.