Port bug 1484118 - replace XPCOM use of nsTransactionManager

RESOLVED FIXED in Thunderbird 63.0

Status

enhancement
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: jorgk, Unassigned)

Tracking

Thunderbird 63.0
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

As per bug 1484118 comment #2 C++ callers can use |new TransactionManager()|.

As per bug 1484118 comment #4 JS callers in Calendar should follow the example in
https://hg.mozilla.org/mozilla-central/rev/58650e51b8b7

Geoff, I'll do the mailnews/ bit, please do the Calendar part.
Flags: needinfo?(geoff)
Masayuki-san, would you mind reviewing this.
Attachment #9003048 - Flags: review?(masayuki)
I've called this nsITransactionManagerExtra, only because it made the copy-and-paste easier.
Flags: needinfo?(geoff)
Attachment #9003058 - Flags: review?(philipp)
Comment on attachment 9003058 [details] [diff] [review]
1485275-transaction-manager-1.diff

Review of attachment 9003058 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks for the quick action!

::: common/src/nsTransactionManagerExtra.cpp
@@ +14,5 @@
> +{
> +    NS_ENSURE_ARG_POINTER(aManager);
> +    nsCOMPtr<nsITransactionManager> manager = new mozilla::TransactionManager();
> +    manager.forget(aManager);
> +    return NS_OK;

Nit: Is this excessive indentation from copy/paste?

::: common/src/nsTransactionManagerExtra.h
@@ +10,5 @@
> +{
> +public:
> +
> +	NS_DECL_THREADSAFE_ISUPPORTS
> +	NS_DECL_NSITRANSACTIONMANAGEREXTRA

And here and below.
Yes, yes it is.
Now without extra spaces (and tabs!).
Attachment #9003058 - Attachment is obsolete: true
Attachment #9003058 - Flags: review?(philipp)
Attachment #9003067 - Flags: review?(philipp)
Comment on attachment 9003048 [details] [diff] [review]
1485275-nsTransactionManager-mailnews.patch

Thanks!
Attachment #9003048 - Flags: review?(masayuki) → review+
Comment on attachment 9003067 [details] [diff] [review]
1485275-transaction-manager-2.diff

r+ on the calendar part, Jörg would probably be better suited to review the common/ changes.
Attachment #9003067 - Flags: review?(philipp)
Attachment #9003067 - Flags: review?(jorgk)
Attachment #9003067 - Flags: review+
Comment on attachment 9003067 [details] [diff] [review]
1485275-transaction-manager-2.diff

Review of attachment 9003067 [details] [diff] [review]:
-----------------------------------------------------------------

I assume you obtained this using |mach uuid|. Looks good, r+ with the comments below considered (as MMD would say ;-)).

::: calendar/base/src/calTransactionManager.js
@@ +7,5 @@
>  function calTransactionManager() {
>      this.wrappedJSObject = this;
>      if (!this.transactionManager) {
>          this.transactionManager =
> +            Components.classes["@mozilla.org/transaction-manager-extra;1"]

Cc. ?

::: common/public/nsITransactionManagerExtra.idl
@@ +4,5 @@
> +
> +#include "nsISupports.idl"
> +#include "nsITransactionManager.idl"
> +
> +interface nsIFile;

Where is that used?

@@ +9,5 @@
> +
> +[scriptable, uuid(d1265f8e-e8ea-482e-9b58-0949cd1d83c0)]
> +interface nsITransactionManagerExtra : nsISupports
> +{
> +    nsITransactionManager createTransactionManager();

More white-space.
Attachment #9003067 - Flags: review?(jorgk) → review+
Keywords: leave-open
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/fd4696b16ee2
Port bug 1484118: replace XPCOM use of nsTransactionManager in mailnews/. r=masayuki
Attachment #9003067 - Attachment is obsolete: true
Attachment #9003315 - Flags: review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/a8bd1c32fd2d
Port bug 1484118: replace XPCOM use of nsTransactionManager in calendar/. r=philipp,jorgk
Status: NEW → RESOLVED
Closed: 11 months ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 63.0
Depends on: 1488677
You need to log in before you can comment on or make changes to this bug.