895009 - Convert on-stack NonNull<T> to T& when passing WebIDL arguments

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

So people don't cargo-cult the NonNull<T> in their signatures.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Don't pass in NonNull/OwningNonNull arguments to WebIDL binding consumers.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 777273 [details] [diff] [review]
Don't pass in NonNull/OwningNonNull arguments to WebIDL binding consumers.

Olli, can you take this, since Peter is on vacation?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 777273 [details] [diff] [review]
Don't pass in NonNull/OwningNonNull arguments to WebIDL binding consumers.

I'd prefer using the normal coding style even in binding templates.
So, aFoo for param names in the new code.

And while you change SurfaceFromElement, want to move { to the next line?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Done, and https://hg.mozilla.org/integration/mozilla-inbound/rev/0d8aa14f5ed3

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Either this patch or the patch from bug 895974 (pushed at the same time) was causing frequent mochitest-1 asserts on the Ubuntu32 test slaves. Backed out.
https://hg.mozilla.org/integration/mozilla-inbound/rev/3794beed0e34

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

13:26:23     INFO -  [Parent 2282] ###!!! ASSERTION: Clock should go forwards if the playback rate is > 0.: 'mCurrentFrameTime <= clock_time || mPlaybackRate <= 0', file ../../../content/media/MediaDecoderStateMachine.cpp, line 2435
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::GetClock() [content/media/MediaDecoderStateMachine.cpp:2434]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::AdvanceFrame() [content/media/MediaDecoderStateMachine.cpp:2462]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::RunStateMachine() [content/media/MediaDecoderStateMachine.cpp:2235]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::CallRunStateMachine() [content/media/MediaDecoderStateMachine.cpp:2741]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::Run() [content/media/MediaDecoderStateMachine.cpp:2715]
13:26:23     INFO -  nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:621]
13:26:23     INFO -  NS_ProcessNextEvent(nsIThread*, bool) [obj-firefox/xpcom/build/nsThreadUtils.cpp:238]
13:26:23     INFO -  nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:249]
13:26:23     INFO -  _pt_root [nsprpub/pr/src/pthreads/ptthread.c:207]
13:26:23     INFO -  libpthread.so.0 + 0x6d4c
13:26:23     INFO -  [Parent 2282] ###!!! ASSERTION: Should have positive clock time.: 'clock_time >= mStartTime', file ../../../content/media/MediaDecoderStateMachine.cpp, line 2462
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::AdvanceFrame() [content/media/MediaDecoderStateMachine.cpp:2462]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::RunStateMachine() [content/media/MediaDecoderStateMachine.cpp:2235]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::CallRunStateMachine() [content/media/MediaDecoderStateMachine.cpp:2741]
13:26:23     INFO -  mozilla::MediaDecoderStateMachine::Run() [content/media/MediaDecoderStateMachine.cpp:2715]
13:26:23     INFO -  nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:621]
13:26:23     INFO -  NS_ProcessNextEvent(nsIThread*, bool) [obj-firefox/xpcom/build/nsThreadUtils.cpp:238]
13:26:23     INFO -  nsThread::ThreadFunc(void*) [xpcom/threads/nsThread.cpp:249]
13:26:23     INFO -  _pt_root [nsprpub/pr/src/pthreads/ptthread.c:207]
13:26:23     INFO -  libpthread.so.0 + 0x6d4c

Comment 6

[Boris Zbarsky [:bzbarsky]]

This wasn't the issue.  Relanded: https://hg.mozilla.org/integration/mozilla-inbound/rev/6ed9c8fc4844

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/6ed9c8fc4844