GeckoView needs to use EventDispatcher.sendResponse

RESOLVED FIXED in mozilla33


Core Graveyard
Embedding: GRE Core
4 years ago
2 years ago


(Reporter: mfinkle, Assigned: mfinkle)



Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)

Created attachment 8449187 [details] [diff] [review]
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.
Attachment #8449187 - Flags: review?(wjohnston)


4 years ago
Blocks: 880107


4 years ago
Duplicate of this bug: 1032207

Comment 2

4 years ago
Is this patch in any of the nightly builds?
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

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/
@@ +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.
Attachment #8449187 - Flags: review?(wjohnston) → review-
Created attachment 8450447 [details] [diff] [review]
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.
Assignee: nobody → mark.finkle
Attachment #8449187 - Attachment is obsolete: true
Attachment #8450447 - Flags: review?(wjohnston)
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?
Attachment #8450447 - Flags: review?(wjohnston) → review+
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.