664695 - Improve signature of nsIConsoleService::GetMessageArray

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[[e-mail]]

The current IDL signature is:
>void getMessageArray([array, size_is(count)] out nsIConsoleMessage messages, out PRUint32 count);
This is inconvenient to use from JavaScript:
>var out = {}; // Throwaway references to support 'out' parameters.
>this.mCService.getMessageArray(out, {});
>var messages = out.value;
Instead, the signature should be changed to:
>void getMessageArray([optional] out PRUint32 count, [retval, array, size_is(count)] out nsIConsoleMessage messages);
This allows the JavaScript to say simply:
>var messages = this.mCService.getMessageArray();

Comment 1

[[e-mail]]

Attachment: Proposed patch

If an extension wanted to be backward compatible, it could write this:
>var out = {}; // Throwaway references to support 'out' parameters.
>var messages = this.mCService.getMessageArray(out, {}) || out.value;

Comment 2

[Benjamin Smedberg]

Comment on attachment 539766 [details] [diff] [review]
Proposed patch

I agree that this is a better signature. I'm not sure it's worth the churn in order to get this better signature now, though. Writeup to dev.platform shortly, but I'm hoping to remove this API and several others in favor of something that looks like DOM events.

Comment 3

[[e-mail]]

Attachment: Updated for bitrot

I changed the forEach in WebConsoleUtils.jsm to a filter because I was touching the code anyway but I can change it back or get separate review on that change.

Comment 4

[Benjamin Smedberg]

Comment on attachment 674600 [details] [diff] [review]
Updated for bitrot

ok. Please talk to jorge about spreading the word to the addon community and adding an addon compat check for whichever release this lands in.

Comment 5

[[e-mail]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/06b998c1100d

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Sorry, but this had to be backed out for mochitest-other orange.
https://hg.mozilla.org/integration/mozilla-inbound/rev/b404d3977390

https://tbpl.mozilla.org/php/getParsedLog.php?id=16701647&tree=Mozilla-Inbound

30056 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/devtools/webconsole/test/test_cached_messages.html | number of cached page errors - got 1, expected 2
30068 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/devtools/webconsole/test/test_cached_messages.html | Test timed out.
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 3942451 bytes during test execution
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of AsyncStatement with size 88 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1760 instances of AtomImpl with size 40 bytes each (70400 bytes total)
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of BackstagePass with size 48 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 1 instance of CalculateFrecencyFunction with size 24 bytes
TEST-UNEXPECTED-FAIL | automationutils.processLeakLog() | leaked 900 instances of CompositeDataSourceImpl with size 120 bytes each (108000 bytes total)

Comment 7

[[e-mail]]

Attachment: Test fix

I couldn't even get the test to work locally without the patch, because it does sneaky things with window.top and doesn't expect the uncaught exception.

Comment 8

[Mozilla RelEng Bot]

Try run for a3794b698647 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a3794b698647
Results (out of 31 total builds):
    success: 30
    warnings: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-a3794b698647

Comment 9

[Mihai Sucan [:msucan]]

Comment on attachment 677967 [details] [diff] [review]
Test fix

This looks good to me. Thank you Neil!

Comment 10

[[e-mail]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9c58f4ae031e

Comment 11

[Mozilla RelEng Bot]

Try run for a3794b698647 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=a3794b698647
Results (out of 32 total builds):
    success: 30
    warnings: 1
    failure: 1
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-a3794b698647

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

https://hg.mozilla.org/mozilla-central/rev/9c58f4ae031e