1033168 - GeckoView needs to use EventDispatcher.sendResponse

Status: RESOLVED FIXED

(Core Graveyard :: Embedding: GRE Core, defect)

Description

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: eventdispatcher-fix v0.1

In bug 946344 we removed GeckoEventResponder and the Prompt:Reply message handling with it. We never updated GeckoView so it's Prompt handling is broken.

The primary way this was found was that the onDebugRequest was failing to allow connections in the GeckoBrowser example project. After getting a patch to convert to EventDispatcher.sendResponse, it's working again.

I'd still like to stop using Prompts for the debugging requests, but that's a different bug.

Comment 2

[dpupek]

Is this patch in any of the nightly builds?

Comment 3

[Wesley Johnston (:wesj)]

Comment on attachment 8449187 [details] [diff] [review]
eventdispatcher-fix v0.1

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

This is not using the old response api. We should use the newer stuff here if we're going to update.

i.e. we should make GeckoView a NativeEventListener
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/EventDispatcher.java#108

and then it will get a callback in its handleMessage function, which you can pass to handlePrompt, which will probably then be handed to the PromptResult callback.

::: mobile/android/base/GeckoView.java
@@ +449,5 @@
>  
>          private JSONObject makeResult(int resultCode) {
>              JSONObject result = new JSONObject();
>              try {
>                  result.put("guid", mGUID);

You don't really need any of this guid stuff anymore either. SendResponse will fill it in for you.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: eventdispatcher-fix v0.2

I talked to Wes on IRC. Converting GeckoView to use NativeEventListener has it's only set of dependenices. This patch removes the "guid" part of the response, but otherwise keeps the same approach.

Comment 5

[Wesley Johnston (:wesj)]

Comment on attachment 8450447 [details] [diff] [review]
eventdispatcher-fix v0.2

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

Mind filing bugs to move things over to the new setup?

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/integration/fx-team/rev/1da6225b0446

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1da6225b0446