Bug 1536047 Comment 64 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

Hi Boris, thanks for your continued support. Please note that the TB project has 12 paid staff now. My roll is to keep the show on the road, fixing bustage, releases, regressions, sheriffing, etc. That said, I can't fix everything that breaks alone. I will mostly get it to the point where we can build, tests are mostly green, so others can work. If that means temporary fixes, switching off stuff or tests, so be it. I keep track of it. So I've handed that stuff over to BenC. If he doesn't fix it very quickly, I'll put JS Account back and disable the static analysis, that's working on beta now.

So for now, I won't be making any more attempts or attaching any patches.

What I said in comment #51:

As far as I can see the compile error for JS Account happens on
2:12.90 In file included from c:/mozilla-source/comm-central/comm/mailnews/jsaccount/src/JaCompose.cpp:7
where the include file `#include "JaCompose.h"` is included. That in turn creates a lot of code in the "forward macros", and as far as I see, the code those macros create would need to be decorated with `MOZ_CAN_RUN_SCRIPT_BOUNDARY`. So personally, I cannot implement your suggestion.

Yes, the preprocessor turns the "forward macros" into what you see in comment #24:
```
#define NS_FORWARD_NSIMSGCOMPOSE(_to) \
  NS_IMETHOD SetDocumentCharset(const char * charset) override { return _to SetDocumentCharset(charset); } \
```
and the `_to` parameter which is substituted by the macro invocation is what the analysis complains about.

So unless I understand it incorrectly, the `NS_IMETHOD SetDocumentCharset` would need to receive the `MOZ_CAN_RUN_SCRIPT_BOUNDARY`, and that's not possible since it's generated by the macro.

>  If you put MOZ_CAN_RUN_SCRIPT_BOUNDARY as close as you can to the relevant callsites
> into Gecko that are now MOZ_CAN_RUN_SCRIPT, the JS Account stuff should not matter
> in any way, I would think. Or are there JS Account impls of core Gecko interfaces 
> (like editor) that are now MOZ_CAN_RUN_SCRIPT? (I sort of doubt that.)

As I said, I don't see any call sites into Gecko-MOZ_CAN_RUN_SCRIPT functions other than the ones I fixed in our compose code. What appears to be happening is that JS Account somehow now forwards to compose interfaces that have been made `MOZ_CAN_RUN_SCRIPT` since they call such Gecko functions. So in a way, yes, JS Account appears to implement `MOZ_CAN_RUN_SCRIPT` functions, but not from Gecko, but from Mailnews.

Wasn't attachment 9051818 [details] [diff] [review] and attachment 9051830 [details] [diff] [review], which both don't compile, attempts to deliver strong references?

No offence intended, but I won't touch it any more for now. I expect to wake up one morning and find this fixed by BenC who is working from NZ (I'm in Europe, we have 12 hours time difference between the two).
Hi Boris, thanks for your continued support. Please note that the TB project has 12 paid staff now. My roll is to keep the show on the road, fixing bustage, releases, regressions, sheriffing, etc. That said, I can't fix everything that breaks alone. I will mostly get it to the point where we can build, tests are mostly green, so others can work. If that means temporary fixes, switching off stuff or tests, so be it. I keep track of it. So I've handed that stuff over to BenC. If he doesn't fix it very quickly, I'll put JS Account back and disable the static analysis, that's working on beta now.

So for now, I won't be making any more attempts or attaching any patches.

What I said in comment #51:

As far as I can see the compile error for JS Account happens on
2:12.90 In file included from c:/mozilla-source/comm-central/comm/mailnews/jsaccount/src/JaCompose.cpp:7
where the include file `#include "JaCompose.h"` is included. That in turn creates a lot of code in the "forward macros", and as far as I see, the code those macros create would need to be decorated with `MOZ_CAN_RUN_SCRIPT_BOUNDARY`. So personally, I cannot implement your suggestion.

Yes, the preprocessor turns the "forward macros" into what you see in comment #24:
```
#define NS_FORWARD_NSIMSGCOMPOSE(_to) \
  NS_IMETHOD SetDocumentCharset(const char * charset) override { return _to SetDocumentCharset(charset); } \
```
and the `_to` parameter which is substituted by the macro invocation is what the analysis complains about.

So unless I understand it incorrectly, the `NS_IMETHOD SetDocumentCharset` would need to receive the `MOZ_CAN_RUN_SCRIPT_BOUNDARY`, and that's not possible since it's generated by the macro.

>  If you put MOZ_CAN_RUN_SCRIPT_BOUNDARY as close as you can to the relevant callsites
> into Gecko that are now MOZ_CAN_RUN_SCRIPT, the JS Account stuff should not matter
> in any way, I would think. Or are there JS Account impls of core Gecko interfaces 
> (like editor) that are now MOZ_CAN_RUN_SCRIPT? (I sort of doubt that.)

As I said, I don't see any call sites into Gecko-MOZ_CAN_RUN_SCRIPT functions other than the ones I fixed in our compose code. What appears to be happening is that JS Account somehow now forwards to compose interfaces that have been made `MOZ_CAN_RUN_SCRIPT` since they call such Gecko functions. So in a way, yes, JS Account appears to implement `MOZ_CAN_RUN_SCRIPT` functions, but not from Gecko, but from Mailnews.

Weren't attachment 9051818 [details] [diff] [review] and attachment 9051830 [details] [diff] [review], which both don't compile, attempts to deliver strong references?

No offence intended, but I won't touch it any more for now. I expect to wake up one morning and find this fixed by BenC who is working from NZ (I'm in Europe, we have 12 hours time difference between the two).

Back to Bug 1536047 Comment 64