Closed Bug 1532261 Opened 2 years ago Closed 2 years ago

merge nsICMSMessage2 and nsICMSMessage into nsICMSMessage

Categories

(MailNews Core :: Security: S/MIME, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 67.0

People

(Reporter: mkmelin, Assigned: mkmelin)

Details

Attachments

(2 files)

For some reason stumbled upon bug 330503...
We should merge nsICMSMessage2 and nsICMSMessage into nsICMSMessage to avoid the strange complications of having two interfaces when it's supposed to be one. Interface stability isn't a thing anymore.

Moved the code into nsICMSMessage
New is infallible, so removed a if (!listener) check too while I was there.

Attachment #9048148 - Flags: review?(kaie)
Status: NEW → ASSIGNED
QA Contact: mkmelin+mozilla
Version: 44 → Trunk
Attachment #9048148 - Flags: review?(kaie) → review+
Keywords: checkin-needed

Could the assignee and reviewer please compile with the patches the submit for landing. We have enough M-C-caused bustage, we don't need bustage of our own :-(

0:35.29 c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/include\nsCOMPtr.h(1158,3): error: static_assert failed due to requirement '!(mozilla::IsSame<nsICMSMessage, nsICMSMessage>::value || mozilla::IsBaseOf<nsICMSMessage, nsICMSMessage>::value)' "don't use do_QueryInterface for compile-time-determinable casts"
0:35.29 static_assert(
0:35.29 ^
0:35.29 c:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/include\nsCOMPtr.h(570,5): note: in instantiation of function template specialization 'nsCOMPtr<nsICMSMessage>::assign_from_qi<nsICMSMessage>' requested here
0:35.29 assign_from_qi(aQI, NS_GET_TEMPLATE_IID(T));
0:35.29 ^
0:35.29 c:/mozilla-source/comm-central/comm/mailnews/mime/src/mimecms.cpp(311,33): note: in instantiation of function template specialization 'nsCOMPtr<nsICMSMessage>::nsCOMPtr<nsICMSMessage>' requested here
0:35.29 nsCOMPtr<nsICMSMessage> msg = do_QueryInterface(aVerifiedMessage);
0:35.29 ^
0:35.29 1 error generated.
0:35.32 mozmake[4]: *** [c:/mozilla-source/comm-central/config/rules.mk:807: mimecms.obj] Error 1
0:35.32 mozmake[4]: *** Waiting for unfinished jobs....
0:36.77 mozmake[3]: *** [c:/mozilla-source/comm-central/config/recurse.mk:74: comm/mailnews/mime/src/target] Error 2
0:36.77 mozmake[3]: *** Waiting for unfinished jobs....

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(kaie)
Keywords: checkin-needed
Comment on attachment 9048148 [details] [diff] [review]
bug1532261_merge_cms.patch

For not even compiling :-(
Attachment #9048148 - Flags: review-

This compiles. Try interdiff.

Flags: needinfo?(mkmelin+mozilla)
Flags: needinfo?(kaie)

Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/664daac5ba9c
merge nsICMSMessage2 and nsICMSMessage into nsICMSMessage. r=kaie,jorgk

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 67.0

Just FYI, the patch does compile just fine on linux. Apparently windows is pickier in this case.
(Yes, the do_QueryInterface is of course pointless there.)

Well, then you should switch compilers or install the static analysis bit. Alternatively, read the code:

-NS_IMETHODIMP nsSMimeVerificationListener::Notify(nsICMSMessage2 *aVerifiedMessage,
+NS_IMETHODIMP nsSMimeVerificationListener::Notify(nsICMSMessage *aVerifiedMessage,
                                                   nsresult aVerificationResultCode)
 {
   // Only continue if we have a valid pointer to the UI
   NS_ENSURE_FALSE(mSinkIsNull, NS_OK);
 
   NS_ENSURE_TRUE(aVerifiedMessage, NS_ERROR_FAILURE);
 
   nsCOMPtr<nsICMSMessage> msg = do_QueryInterface(aVerifiedMessage);

You've changed the interface, so the QI was unneeded.

I'm just compiling with what mozilla's setup gives me.

You need to log in before you can comment on or make changes to this bug.