Closed Bug 1323429 Opened 7 years ago Closed 7 years ago

[geckoview] [e10s] Make IME work

Categories

(Core Graveyard :: Embedding: APIs, defect)

defect
Not set
normal

Tracking

(firefox54 fixed)

RESOLVED FIXED
mozilla54
Tracking Status
firefox54 --- fixed

People

(Reporter: esawin, Assigned: esawin)

References

Details

Attachments

(2 files, 3 obsolete files)

GV crashes on user input with e10s enabled.
This is a workaround to prevent it from crashing since IME{StateManager|ContentObserver} run in a different process.
Attachment #8830010 - Flags: feedback?(nchen)
Comment on attachment 8830010 [details] [diff] [review]
0001-Bug-1323429-1.0-Remove-invalid-cross-process-calls.patch

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

We will regress IME if we remove these checks. Does IMEStateManager::GetFocusSelectionAndRoot fail? If so, maybe you can skip these checks only if IMEStateManager::GetFocusSelectionAndRoot fails (in which case we assume we are running under e10s).
Attachment #8830010 - Flags: feedback?(nchen)
I assume this will regress some use cases, but could be OK as a temporary workaround.

I'm assigning this bug to myself only to address some of the current IME blockers to allow for GV testing.
Assignee: nobody → esawin
Attachment #8830010 - Attachment is obsolete: true
Attachment #8830080 - Flags: review?(nchen)
Sorry, that's the correct patch version.
Attachment #8830080 - Attachment is obsolete: true
Attachment #8830080 - Flags: review?(nchen)
Attachment #8830082 - Flags: review?(nchen)
Attachment #8830082 - Flags: review?(nchen) → review+
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/902da67acc3c
[1.1] Don't verify IME root content with e10s enabled. r=jchen
Keywords: leave-open
Comment on attachment 8830508 [details] [diff] [review]
0002-Bug-1323429-2.0-Remove-the-context-reference-from-Cl.patch

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

::: mobile/android/base/moz.build
@@ +305,4 @@
>      'sqlite/SQLiteBridge.java',
>      'sqlite/SQLiteBridgeException.java',
>      'TouchEventInterceptor.java',
> +    'util/Clipboard.java',

We should move it out of util/

::: mobile/android/geckoview/src/main/java/org/mozilla/gecko/util/Clipboard.java
@@ +7,4 @@
>  import java.util.concurrent.SynchronousQueue;
>  
>  import org.mozilla.gecko.annotation.WrapForJNI;
> +import org.mozilla.gecko.AppConstants.Versions;

Not used?

@@ +55,4 @@
>              public void run() {
>                  // In API Level 11 and above, CLIPBOARD_SERVICE returns android.content.ClipboardManager,
>                  // which is a subclass of android.text.ClipboardManager.
> +                Context context = GeckoAppShell.getContext();

final, and use GeckoAppShell.getApplicationContext

@@ +58,5 @@
> +                Context context = GeckoAppShell.getContext();
> +                if (context == null) {
> +                    return;
> +                }
> +                final android.content.ClipboardManager cm = (android.content.ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE);

Import android.content.ClipboardManager

@@ +78,4 @@
>       */
>      @WrapForJNI(calledFrom = "gecko")
>      public static boolean hasText() {
> +        Context context = GeckoAppShell.getContext();

Ditto

@@ +81,5 @@
> +        Context context = GeckoAppShell.getContext();
> +        if (context == null) {
> +            return false;
> +        }
> +        android.content.ClipboardManager cm = (android.content.ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE);

final

@@ +101,4 @@
>       */
>      @SuppressWarnings("deprecation")
>      static String getClipboardTextImpl() {
> +        Context context = GeckoAppShell.getContext();

Ditto

@@ +104,5 @@
> +        Context context = GeckoAppShell.getContext();
> +        if (context == null) {
> +            return null;
> +        }
> +        android.content.ClipboardManager cm = (android.content.ClipboardManager) context.getSystemService(Context.CLIPBOARD_SERVICE);

final
Attachment #8830508 - Flags: review?(nchen) → review+
Addressed comments.
Attachment #8830508 - Attachment is obsolete: true
Attachment #8830724 - Flags: review+
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/08a6ef48cebd
[2.1] Remove the context reference from Clipboard. r=jchen
Keywords: leave-open
https://hg.mozilla.org/mozilla-central/rev/08a6ef48cebd
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: