Closed
Bug 664695
Opened 13 years ago
Closed 12 years ago
Improve signature of nsIConsoleService::GetMessageArray
Categories
(Core :: XPCOM, enhancement)
Core
XPCOM
Tracking
()
RESOLVED
FIXED
mozilla19
People
(Reporter: neil, Assigned: neil)
Details
(Keywords: addon-compat)
Attachments
(2 files, 1 obsolete file)
10.43 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
11.86 KB,
patch
|
msucan
:
review+
|
Details | Diff | Splinter Review |
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();
Assignee | ||
Comment 1•13 years ago
|
||
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•13 years ago
|
||
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.
Updated•13 years ago
|
Attachment #539766 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 3•12 years ago
|
||
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.
Attachment #539766 -
Attachment is obsolete: true
Attachment #674600 -
Flags: review?(benjamin)
Comment 4•12 years ago
|
||
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.
Attachment #674600 -
Flags: review?(benjamin) → review+
Updated•12 years ago
|
Keywords: addon-compat
Assignee | ||
Comment 5•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/06b998c1100d
Comment 6•12 years ago
|
||
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)
Assignee | ||
Comment 7•12 years ago
|
||
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.
Attachment #677967 -
Flags: review?(rcampbell)
Attachment #677967 -
Flags: review?(mihai.sucan)
Comment 8•12 years ago
|
||
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/neil@parkwaycc.co.uk-a3794b698647
Comment 9•12 years ago
|
||
Comment on attachment 677967 [details] [diff] [review] Test fix This looks good to me. Thank you Neil!
Attachment #677967 -
Flags: review?(mihai.sucan) → review+
Assignee | ||
Updated•12 years ago
|
Attachment #677967 -
Flags: review?(rcampbell)
Assignee | ||
Comment 10•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/9c58f4ae031e
Comment 11•12 years ago
|
||
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/neil@parkwaycc.co.uk-a3794b698647
Comment 12•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9c58f4ae031e
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in
before you can comment on or make changes to this bug.
Description
•