1035595 - Don't use specific type for nsTArray<T>::Length() in nsGUIEventIPC.h

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

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

The result of nsTArray<T>::Length() is defined as nsTArray<T>::size_type. It was alias of uint32_t. However, now, size_t.

nsGUIEventIPC.h is too sensitive for size difference of it. Let's use logically correct type.

Comment 1

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

Attachment: Use strictly same type variable at reading length of nsTArray derived classes in nsGUIEventIPC.h

Comment 2

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

Attachment: Use mozilla::WidgetTouchEvent::TouchArray instead of nsTArry<nsRefPtr<mozilla::dom::Touch>>

This is optional patch. This replaces nsTArray<nsRefPtr<mozilla::dom::Touch>> with WidgetTouchEvent::TouchArray. This is not related this bug directly, however, this improves the code with the new type for easier to maintain.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 8452191 [details] [diff] [review]
Use mozilla::WidgetTouchEvent::TouchArray instead of nsTArry<nsRefPtr<mozilla::dom::Touch>>

Review of attachment 8452191 [details] [diff] [review]:
-----------------------------------------------------------------

You could use "auto" in some of these places too.

Comment 4

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

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #3)
> You could use "auto" in some of these places too.

Indeed. I'll file a bug for it. Thank you.

Comment 5

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/c45fdb6a9e35
https://hg.mozilla.org/integration/mozilla-inbound/rev/c175c61ec086

The first patch fixes a crash bug in e10s build. I'll request approval for 32 since it's a regression of bug 1004098.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c45fdb6a9e35
https://hg.mozilla.org/mozilla-central/rev/c175c61ec086

Comment 7

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

Comment on attachment 8452186 [details] [diff] [review]
Use strictly same type variable at reading length of nsTArray derived classes in nsGUIEventIPC.h

Approval Request Comment
[Feature/regressing bug #]: bug 1004098
[User impact if declined]: In e10s mode on 64bit binary, this may cause crash (by MOZ_CRASH() in editor) occurs on 64 bit binary at using IME. (sizeof(size_t) should be 64bit on 64bit buils, therefore, at writing the length of the array in WidgetTextEvent, it writes as uint64_t. However, at reading it, it picks up as uint32_t. This caused reading wrong address at reading remaining members.
[Describe test coverage new/current, TBPL]: Not tested with automated tests. This is already landed on m-c.
[Risks and why]: None because this just makes nsGUIEventIPC.h use strictly same variable type of the result of nsTArray<T>::Length() at reading its value in child process.
[String/UUID change made/needed]: Nothing.

Comment 8

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8452186 [details] [diff] [review]
Use strictly same type variable at reading length of nsTArray derived classes in nsGUIEventIPC.h

Given that this is e10s specific and we don't ship e10s, Aurora-