If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Add GeckoView content module registration mechanics

RESOLVED FIXED in Firefox 56

Status

()

Firefox for Android
GeckoView
RESOLVED FIXED
2 months ago
2 months ago

People

(Reporter: esawin, Assigned: esawin)

Tracking

51 Branch
Firefox 56
All
Android
Points:
---

Firefox Tracking Flags

(firefox56 fixed)

Details

Attachments

(5 attachments, 3 obsolete attachments)

(Assignee)

Description

2 months ago
With bug 1372681, we enabled registration of GeckoView modules to avoid handling events which have no active listeners.

In this bug we want to extend the same mechanics to frame scripts which are loaded during a GeckoView module's initialization to avoid handling content events without active listeners.
(Assignee)

Comment 1

2 months ago
Created attachment 8885383 [details] [diff] [review]
0001-Bug-1380071-1.0-Add-base-class-for-GeckoView-content.patch
Attachment #8885383 - Flags: review?(nchen)
(Assignee)

Comment 2

2 months ago
Created attachment 8885384 [details] [diff] [review]
0002-Bug-1380071-2.0-Use-content-module-base-class-for-th.patch
Attachment #8885384 - Flags: review?(nchen)
(Assignee)

Comment 3

2 months ago
Created attachment 8885385 [details] [diff] [review]
0003-Bug-1380071-3.0-Use-content-module-base-class-for-th.patch
Attachment #8885385 - Flags: review?(nchen)
(Assignee)

Comment 4

2 months ago
Created attachment 8885386 [details] [diff] [review]
0004-Bug-1380071-4.0-Delay-frame-script-loading-to-GeckoV.patch
Attachment #8885386 - Flags: review?(nchen)
(Assignee)

Comment 5

2 months ago
Created attachment 8885387 [details] [diff] [review]
0005-Bug-1380071-5.0-Delay-frame-script-loading-to-GeckoV.patch
Attachment #8885387 - Flags: review?(nchen)
Attachment #8885383 - Flags: review?(nchen) → review+
Comment on attachment 8885384 [details] [diff] [review]
0002-Bug-1380071-2.0-Use-content-module-base-class-for-th.patch

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

::: mobile/android/chrome/geckoview/GeckoViewContent.js
@@ +25,2 @@
>  
>      addMessageListener("GeckoView:DOMFullscreenEntered", this);

These calls should use whatever message manager that was passed to the class.
Attachment #8885384 - Flags: review?(nchen) → review+
Comment on attachment 8885385 [details] [diff] [review]
0003-Bug-1380071-3.0-Use-content-module-base-class-for-th.patch

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

::: mobile/android/chrome/geckoview/GeckoViewScrollContent.js
@@ +14,5 @@
>    // dump(aMsg);
>  }
>  
> +class GeckoViewScrollContent extends GeckoViewContentModule {
> +  init() {

Don't need this since we're not doing anything here
Attachment #8885385 - Flags: review?(nchen) → review+
Attachment #8885386 - Flags: review?(nchen) → review+
Attachment #8885387 - Flags: review?(nchen) → review+
(Assignee)

Comment 8

2 months ago
Created attachment 8886132 [details] [diff] [review]
0001-Bug-1380071-1.1-Add-base-class-for-GeckoView-content.patch

Addressed comments.
Attachment #8885383 - Attachment is obsolete: true
Attachment #8886132 - Flags: review+
(Assignee)

Comment 9

2 months ago
Created attachment 8886134 [details] [diff] [review]
0002-Bug-1380071-2.1-Use-content-module-base-class-for-th.patch

Addressed comments and added missing unregister() override.
Attachment #8885384 - Attachment is obsolete: true
(Assignee)

Comment 10

2 months ago
Created attachment 8886135 [details] [diff] [review]
0003-Bug-1380071-3.1-Use-content-module-base-class-for-th.patch

Addressed comments.
Attachment #8885385 - Attachment is obsolete: true
Attachment #8886135 - Flags: review+
(Assignee)

Updated

2 months ago
Attachment #8886134 - Flags: review+

Comment 11

2 months ago
Pushed by esawin@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/13a43361f217
[1.1] Add base class for GeckoView content modules. r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/99ca35b6467b
[2.1] Use content module base class for the GeckoView content module. r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ef8c2565cb3
[3.1] Use content module base class for the GeckoView scroll module. r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/852855d5d2cf
[4.0] Delay frame script loading to GeckoView content module registration. r=jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/da0754f7bb57
[5.0] Delay frame script loading to GeckoView scroll module registration. r=jchen

Comment 12

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/13a43361f217
https://hg.mozilla.org/mozilla-central/rev/99ca35b6467b
https://hg.mozilla.org/mozilla-central/rev/9ef8c2565cb3
https://hg.mozilla.org/mozilla-central/rev/852855d5d2cf
https://hg.mozilla.org/mozilla-central/rev/da0754f7bb57
Status: NEW → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
You need to log in before you can comment on or make changes to this bug.