Closed Bug 1433008 Opened 2 years ago Closed 2 years ago

Make WidgetEvent movable


(Core :: Widget, enhancement)

Not set



Tracking Status
firefox60 --- fixed


(Reporter: hiro, Assigned: hiro)



(2 files)

For animation events handling, we are using InternalTransitionEvent and InternalAnimationEvent, which are inheritances of WidgetEvent, in nsTArray<> and sorting them when we dispatch the events. 

It would be nice to have move constructor and move assignment for WidgetEvent to prevent copying them in the sort.

Though I haven't checked other inheritances, there might be other situations that can avoid copying.
A try;

It didn't finish yet, but looks good so far.

One thing I am concerned is that Brian noted that std::stable_sort() is not move friendly on MacOSX and Android in bug 1263500  two years ago.  Unfortunately our CI uses cross compile for OSX so I can't confirm whether it still uses copy constructor or not.  I appreciate someone tries to build with the patches in the try on OSX.  (Whereas on Android the build is fine on the try)
:arai did confirm the patch can be built with XCode clang on MacOSX.  So I think we can land them now. :)
Comment on attachment 8946499 [details]
Bug 1433008 - Make WidgetEvent movable.
Attachment #8946499 - Flags: review?(masayuki) → review+
Comment on attachment 8946500 [details]
Bug 1433008 - Make AnimationEventInfo, InternalTransitionEvent and InternalAnimationEvent movable.
Attachment #8946500 - Flags: review?(masayuki) → review+
Thank you!

A try run; can be built on mingw. :)
Assignee: nobody → hikezoe
Pushed by
Make WidgetEvent movable. r=masayuki
Make AnimationEventInfo, InternalTransitionEvent and InternalAnimationEvent movable. r=masayuki
Ok, so only the basic WidgetEvent is moveable.
Is there any way we could ensure new member variables are added to the move-ctor, and not only to other ctors?
Hmm... I don't know.  How have we ever avoided such mistakes?
At least add some comments somewhere?
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Thanks. Sounds reasonable.  I will add comments in a later bug.  This bug was now closed. :)
Filed bug 1434220.  I will work on it tomorrow.
You need to log in before you can comment on or make changes to this bug.