Closed
Bug 1167423
Opened 9 years ago
Closed 9 years ago
Handle return values of FallibleTArray functions in dom/
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla41
Tracking | Status | |
---|---|---|
firefox41 | --- | fixed |
People
(Reporter: poiru, Assigned: baku)
References
(Blocks 1 open bug)
Details
Attachments
(10 files, 1 obsolete file)
11.92 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
892 bytes,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
3.48 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
7.16 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
7.69 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
755 bytes,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.87 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
1.96 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
10.88 KB,
patch
|
jya
:
review+
|
Details | Diff | Splinter Review |
1.52 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
Below is a list of unchecked fallible calls. After bug 968520 lands, these will cause 'unused result' warnings. Could you please let me know how you'd like them fixed (e.g. ignore result, use MOZ_ASSERT/MOZ_ENSURE_TRUE, propagate error)? Feel free to fix them yourself. dom/base/Console.cpp:1430:49: Dmitry Sagalovskiy: aSequence.AppendElement(JS::StringValue(str)); dom/base/Console.cpp:1555:34: Andrea Marchesini: aSequence.AppendElement(v); dom/base/Console.cpp:1573:52: Mihai Sucan: aStyles.AppendElement(JS::NullValue()); dom/base/Console.cpp:1576:58: Mihai Sucan: aStyles.AppendElement(JS::StringValue(jsString)); dom/base/Console.cpp:1644:41: Andrea Marchesini: aSequence.AppendElement(aData[index]); dom/base/Console.cpp:1776:37: Andrea Marchesini: aSequence.AppendElement(aData[i]); dom/base/Console.cpp:669:36: Andrea Marchesini: arguments.AppendElement(value); dom/base/Console.cpp:830:29: Andrea Marchesini: data.AppendElement(aTime); dom/base/Console.cpp:843:29: Andrea Marchesini: data.AppendElement(aTime); dom/base/Console.cpp:856:29: Jordan Santell: data.AppendElement(aData); dom/base/Console.cpp:895:36: Andrea Marchesini: sequence.AppendElement(aData[i]); dom/base/WebSocket.cpp:963:36: Andrea Marchesini: protocols.AppendElement(aProtocol); dom/base/nsDOMMutationObserver.cpp:715:73: Olli Pettay: filtersAsStrings.AppendElement(nsDependentAtomString(filters[j])); dom/canvas/CanvasRenderingContext2D.cpp:2677:25: Rik Cabanier: dash.AppendElement(1); dom/canvas/CanvasRenderingContext2D.cpp:2678:25: Rik Cabanier: dash.AppendElement(1); dom/canvas/CanvasRenderingContext2D.cpp:3960:36: Rik Cabanier: dash.AppendElement(aSegments[x]); dom/canvas/CanvasRenderingContext2D.cpp:3964:38: Rik Cabanier: dash.AppendElement(aSegments[x]); dom/canvas/WebGL2ContextFramebuffers.cpp:352:62: David Anderson: out->AppendElement(LOCAL_GL_COLOR_ATTACHMENT0); dom/canvas/WebGL2ContextFramebuffers.cpp:355:61: David Anderson: out->AppendElement(LOCAL_GL_DEPTH_ATTACHMENT); dom/canvas/WebGL2ContextFramebuffers.cpp:358:63: David Anderson: out->AppendElement(LOCAL_GL_STENCIL_ATTACHMENT); dom/crypto/WebCryptoTask.cpp:1967:56: Richard Barnes: mJwk.mKey_ops.Value().AppendElements(mKeyUsages); dom/datastore/DataStoreDB.cpp:319:69: Ben Turner: objectStores.RawSetAsStringSequence().AppendElements(mObjectStores); dom/datastore/DataStoreService.cpp:1361:60: Andrea Marchesini: dbs.AppendElement(NS_LITERAL_STRING(DATASTOREDB_REVISION)); dom/html/HTMLInputElement.cpp:1863:32: Andrea Marchesini: list.AppendElement(aValue); dom/html/HTMLInputElement.cpp:2444:56: Andrea Marchesini: list.AppendElement(nsDependentString(aFileNames[i])); dom/html/HTMLInputElement.cpp:2495:30: Andrea Marchesini: list.AppendElement(aValue); dom/media/mediasource/ResourceQueue.cpp:115:58: Chris Double: data->AppendElements(item->mData->Elements() + offset, dom/mobilemessage/MobileMessageManager.cpp:602:71: Bevis Tseng: deletedMsgIds.AppendElements(info->GetData().deletedMessageIds()); dom/mobilemessage/MobileMessageManager.cpp:608:73: Bevis Tseng: deletedThreadIds.AppendElements(info->GetData().deletedThreadIds()); dom/tv/TVSource.cpp:352:29: Sean Lin: *programs.AppendElement() = program;
Comment 1•9 years ago
|
||
(In reply to Birunthan Mohanathas [:poiru] from comment #0) > dom/base/nsDOMMutationObserver.cpp:715:73: > Olli Pettay: > filtersAsStrings.AppendElement(nsDependentAtomString(filters[j])); The best option would be to mark http://mxr.mozilla.org/mozilla-central/source/dom/webidl/MutationObserver.webidl?rev=98a32495dbb1&mark=46-47#47 [ChromeOnly,Throws] and then nsDOMMutationObserver::GetObservingInfo gets ErrorResult& as the second param, and pass NS_ERROR_OUT_OF_MEMORY to it.
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8609290 -
Flags: review?(bugs)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → amarchesini
Assignee | ||
Comment 3•9 years ago
|
||
Attachment #8609294 -
Flags: review?(bugs)
Assignee | ||
Comment 4•9 years ago
|
||
Attachment #8609294 -
Attachment is obsolete: true
Attachment #8609294 -
Flags: review?(bugs)
Attachment #8609298 -
Flags: review?(bugs)
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8609301 -
Flags: review?(bugs)
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8609303 -
Flags: review?(bugs)
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8609308 -
Flags: review?(bugs)
Assignee | ||
Comment 8•9 years ago
|
||
Attachment #8609313 -
Flags: review?(bugs)
Assignee | ||
Comment 9•9 years ago
|
||
Attachment #8609316 -
Flags: review?(bugs)
Assignee | ||
Comment 10•9 years ago
|
||
Attachment #8609317 -
Flags: review?(bugs)
Assignee | ||
Comment 11•9 years ago
|
||
Attachment #8609318 -
Flags: review?(bugs)
Assignee | ||
Comment 12•9 years ago
|
||
Attachment #8609319 -
Flags: review?(bugs)
Updated•9 years ago
|
Attachment #8609290 -
Flags: review?(bugs) → review+
Updated•9 years ago
|
Attachment #8609298 -
Flags: review?(bugs) → review+
Comment 13•9 years ago
|
||
Comment on attachment 8609301 [details] [diff] [review] patch 3 - MutationObserver >-nsDOMMutationObserver::GetObservingInfo(nsTArray<Nullable<MutationObservingInfo> >& aResult) >+nsDOMMutationObserver::GetObservingInfo( >+ nsTArray<Nullable<MutationObservingInfo>>& aResult, >+ mozilla::ErrorResult& aRv) a bit odd indentation. Perhaps >+nsDOMMutationObserver::GetObservingInfo( >+ nsTArray<Nullable<MutationObservingInfo>>& aResult, >+ mozilla::ErrorResult& aRv) though that doesn't look too good either (but would at least use 2 spaces from something). Either way.
Attachment #8609301 -
Flags: review?(bugs) → review+
Updated•9 years ago
|
Attachment #8609319 -
Flags: review?(bugs) → review+
Comment 14•9 years ago
|
||
I'm not sure about adding [Throws] to .webidl in case the API is non-ChromeOnly.
Updated•9 years ago
|
Attachment #8609316 -
Flags: review?(bugs) → review+
Updated•9 years ago
|
Attachment #8609317 -
Flags: review?(bugs) → review+
Updated•9 years ago
|
Attachment #8609313 -
Flags: review?(bugs) → review+
Comment 15•9 years ago
|
||
Comment on attachment 8609318 [details] [diff] [review] patch 9 - MediaSource (I don't know what behavior is wanted here.)
Attachment #8609318 -
Flags: review?(bugs) → review?(jyavenard)
Comment 16•9 years ago
|
||
Comment on attachment 8609303 [details] [diff] [review] patch 4 - CanvasRenderingContext2D >+ if (!dash.AppendElement(aSegments[x])) { >+ aRv.Throw(NS_ERROR_OUT_OF_MEMORY); missing indentation though, I'm not really agains throws.
Attachment #8609303 -
Flags: review?(bugs) → review+
Updated•9 years ago
|
Attachment #8609308 -
Flags: review?(bugs) → review+
Comment 17•9 years ago
|
||
Comment on attachment 8609318 [details] [diff] [review] patch 9 - MediaSource Review of attachment 8609318 [details] [diff] [review]: ----------------------------------------------------------------- nice catch. We need to report the error to the owning's sourcebuffer. There's no easy implementation nor do I think the MSE spec cater for this (appending to a source buffer causing another sourcebuffer to abort). But seeing that particular eviction isn't currently working and that code isn't called. will be fine for now. Please open a follow up bug to handle this. thanks ::: dom/media/mediasource/TrackBuffer.cpp @@ +347,4 @@ > toEvict -= decoders[i]->GetResource()->EvictData(playbackOffset, > + playbackOffset, > + rv); > + if (NS_WARN_IF(rv.Failed())) { trailing space @@ +500,5 @@ > + mInitializedDecoders[i]->GetResource()->EvictBefore(endOffset, rv); > + if (NS_WARN_IF(rv.Failed())) { > + rv.SuppressException(); > + return; > + } this will leave data in an unplayable state, we need a way to report that error back to the owning element and properly throw the error to the owning sourcebuffer.
Attachment #8609318 -
Flags: review?(jyavenard) → review+
Assignee | ||
Comment 18•9 years ago
|
||
remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/fc7b8402d18b remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/84ea22e04a62 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/c85d3209f157 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/a0dda76be94a remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/e0946c62bd88 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/5bc009d44304 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/f75a03f1e8b2 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/02d0f71e3b76 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/cfe431411b3d remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/0fd1df73aa8b
Comment 19•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/fc7b8402d18b https://hg.mozilla.org/mozilla-central/rev/84ea22e04a62 https://hg.mozilla.org/mozilla-central/rev/c85d3209f157 https://hg.mozilla.org/mozilla-central/rev/a0dda76be94a https://hg.mozilla.org/mozilla-central/rev/e0946c62bd88 https://hg.mozilla.org/mozilla-central/rev/5bc009d44304 https://hg.mozilla.org/mozilla-central/rev/f75a03f1e8b2 https://hg.mozilla.org/mozilla-central/rev/02d0f71e3b76 https://hg.mozilla.org/mozilla-central/rev/cfe431411b3d https://hg.mozilla.org/mozilla-central/rev/0fd1df73aa8b
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•