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

RESOLVED FIXED in Firefox 67

Status

defect
RESOLVED FIXED
6 months ago
2 months ago

People

(Reporter: masayuki, Assigned: bzbarsky)

Tracking

(Blocks 1 bug)

Trunk
mozilla67

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments)

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
Summary: Cannot set nullptr to ref-countable argument if the method is marked as MOZ_CAN_RUN_SCRIPT → Cannot set ref-countable argument to nullptr if the method is marked as MOZ_CAN_RUN_SCRIPT

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...

(Assignee)

Updated

2 months ago
Flags: needinfo?(masayuki)

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

Flags: needinfo?(masayuki)
Attachment #9050169 - Attachment description: Bug 1506439. Fix CanRunScript analysis handling of arguments that default to null. r=andi → Bug 1506439 part 1. Fix CanRunScript analysis handling of arguments that default to null. r=andi

Comment 5

2 months ago
Pushed by bzbarsky@mozilla.com:
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
(Assignee)

Updated

2 months ago
Assignee: nobody → bzbarsky

Comment 6

2 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.