1485275 - Port bug 1484118 - replace XPCOM use of nsTransactionManager

Status: RESOLVED FIXED

(Thunderbird :: General, enhancement)

Description

[Jorg K (CEST = GMT+2)]

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.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1485275-nsTransactionManager-mailnews.patch

Masayuki-san, would you mind reviewing this.

Comment 2

[Geoff Lankow (:darktrojan)]

Attachment: 1485275-transaction-manager-1.diff

I've called this nsITransactionManagerExtra, only because it made the copy-and-paste easier.

Comment 3

[Jorg K (CEST = GMT+2)]

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.

Comment 4

[Geoff Lankow (:darktrojan)]

Yes, yes it is.

Comment 5

[Geoff Lankow (:darktrojan)]

Attachment: 1485275-transaction-manager-2.diff

Now without extra spaces (and tabs!).

Comment 6

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 9003048 [details] [diff] [review]
1485275-nsTransactionManager-mailnews.patch

Thanks!

Comment 7

[Philipp Kewisch [:Fallen][📱📆]]

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.

Comment 8

[Jorg K (CEST = GMT+2)]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fd4696b16ee2
Port bug 1484118: replace XPCOM use of nsTransactionManager in mailnews/. r=masayuki

Comment 10

[Geoff Lankow (:darktrojan)]

Attachment: 1485275-transaction-manager-3.diff

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/a8bd1c32fd2d
Port bug 1484118: replace XPCOM use of nsTransactionManager in calendar/. r=philipp,jorgk