Closed Bug 1339685 Opened 3 years ago Closed 3 years ago

Split GeckoEditable into parent and child classes

Categories

(Firefox for Android :: Keyboards and IME, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 54
Tracking Status
firefox54 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

Details

Attachments

(8 files)

Split GeckoEditable into parent and child classes, with the parent class living inside the main process, and the child class living inside the main process and any child content processes.
Specify a list of AIDL files for GeckoView so we can include AIDLs from
multiple packages, and not just those from the org.mozilla.gecko.process
package.
Attachment #8840573 - Flags: review?(nalexander)
Add IGeckoEditableParent.aidl and IGeckoEditableChild.aidl for two-way
communication between the parent, which lives in the main process, and
the child, which lives in the main process or a child content process.
Attachment #8840574 - Flags: review?(esawin)
Auto-generate native constants for the constants in GeckoEditableClient,
instead of keeping a separate set of constants in native code.
Attachment #8840591 - Flags: review?(esawin)
Add the GeckoEditableChild class, which is currently only used in the
main process as the interface between the native nsWindow and
GeckoEditable. Eventually, it will be expanded to child content
processes as the interface between the native PuppetWidget and
main process GeckoEditable.
Attachment #8840593 - Flags: review?(esawin)
Make calls to GeckoEditableChild from GeckoEditable, and remove code
that exists in GeckoEditableChild from GeckoEditable.
Attachment #8840594 - Flags: review?(esawin)
Add a convenience function for getting the C++ object that is the target
of the native call.
Attachment #8840595 - Flags: review?(snorp)
Make nsWindow and GeckoEditableSupport use GeckoEditableChild for
communication. nsWindow still keeps a reference to GeckoEditable for
switching views.
Attachment #8840596 - Flags: review?(esawin)
Attachment #8840574 - Attachment description: 2. Add AIDLs for GeckoEditable (v2) → 2. Add AIDLs for GeckoEditable (v1)
Comment on attachment 8840573 [details] [diff] [review]
1. Support compiling GeckoView aidl from multiple packages (v1)

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

Sure.
Attachment #8840573 - Flags: review?(nalexander) → review+
Attachment #8840595 - Flags: review?(snorp) → review+
Attachment #8840574 - Flags: review?(esawin) → review+
Attachment #8840591 - Flags: review?(esawin) → review+
Attachment #8840593 - Flags: review?(esawin) → review+
Attachment #8840594 - Flags: review?(esawin) → review+
Comment on attachment 8840596 [details] [diff] [review]
7. Use GeckoEditableChild from native code (v1)

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

::: widget/android/GeckoEditableSupport.h
@@ +130,5 @@
>              {
> +                using GES = GeckoEditableSupport;
> +                return Base::lambda.IsTarget(&GES::OnKeyEvent) ||
> +                        Base::lambda.IsTarget(&GES::OnImeReplaceText) ||
> +                        Base::lambda.IsTarget(&GES::OnImeUpdateComposition) ?

Can you add parentheses for readability?
Attachment #8840596 - Flags: review?(esawin) → review+
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/952cd4007da7
Split GeckoEditable into parent and child classes; r=nalexander r=esawin r=snorp
https://hg.mozilla.org/mozilla-central/rev/952cd4007da7
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 54
No longer blocks: 1359983
You need to log in before you can comment on or make changes to this bug.