1506439 - Cannot set ref-countable argument to nullptr if the method is marked as MOZ_CAN_RUN_SCRIPT

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

If there is like this method in nsContentUtils:

MOZ_CAN_RUN_SCRIPT static void DoSomething(TextEditor* aTextEditor = nullptr);

Then, nsContentUtils::DoSomething(); becomes bustage due to:
>  error: arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)

Here is an example:
https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=210861923&revision=d07307d5ff5be5d35dfd3b2df9d80bde47c36a71
> [task 2018-11-09T17:43:11.531Z] 17:43:11     INFO -  /builds/worker/workspace/build/src/dom/html/HTMLInputElement.cpp:259:19: error: arguments must all be strong refs or parent parameters when calling a function marked as MOZ_CAN_RUN_SCRIPT (including the implicit object argument)
> [task 2018-11-09T17:43:11.531Z] 17:43:11     INFO -      nsresult rv = nsContentUtils::DispatchInputEvent(inputElement);
> [task 2018-11-09T17:43:11.531Z] 17:43:11     INFO -                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And this is fixed by doing:

RefPtr<TextEditor> textEditor;
nsContentUtils::DoSomething(textEditor);

https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=210861923&revision=13feb0dd4081f578f811ca9a99e43081ffc6940b

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1506439 part 1. Fix CanRunScript analysis handling of arguments that default to null. r=andi

Comment 2

[Boris Zbarsky [:bzbarsky]]

Masayuki, how did you intend to write nsContentUtils::DispatchInputEvent once this is fixed? Is the idea to do:

  MOZ_CAN_RUN_SCRIPT
  static nsresult DispatchInputEvent(Element* aEventTarget) {
    return DispatchInputEvent(aEventTarget, mozilla::EditorInputType::eUnknown,
                              nullptr, InputEventOptions());
  }

(which I think can be done without this fix too)? Or to actually use a default arg? Back when this bug was filed we didn't have the EditorInputType and InputEventOptions args, looks like...

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I think that the overload should be used when there is no TextEditor instance. So, your code example looks fine to me.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: Bug 1506439 part 2. Stop creating a useless nsCOMPtr in DispatchInputEvent. r=masayuki

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9b6190dc000d
part 1.  Fix CanRunScript analysis handling of arguments that default to null.  r=andi
https://hg.mozilla.org/integration/autoland/rev/fcaf6f3d0497
part 2.  Stop creating a useless nsCOMPtr in DispatchInputEvent.  r=masayuki

Comment 6

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/9b6190dc000d
https://hg.mozilla.org/mozilla-central/rev/fcaf6f3d0497