Closed Bug 1675644 Opened 4 years ago Closed 4 years ago

WebExtensionController::mPendingMessages should be per-native app

Categories

(GeckoView :: Extensions, defect)

Product:
GeckoView
Component:
Extensions
Platform:
Unspecified
All
Type:
defect

Tracking

(firefox84 fixed)

Status:
RESOLVED FIXED
Milestone:
84 Branch
Tracking Flags:
Tracking Status
firefox84 --- fixed

People

(Reporter: agi, Assigned: agi)

Details

Attachments

(1 file)

Bug 1675644 - Flush extension messages per-session and per-nativeApp.
47 bytes, text/x-phabricator-request
Details | Review

WebExtensionController::mPendingMessages is currently grouped by extension, which means that when an embedder sets the MessageDelegate for a native app, all messages for the extension are flushed, regardless of whether the native app has a delegate or not.

Instead, we should make mPendingMessages per-native app so that only the relevant messages are flushed.

Assignee: nobody → agi

The extension code tries to flush messages when the relevant delegate is attached.

The logic, however, is pretty flawed: we currently only flush runtime-messages
(i.e. not coming from a WebExtension Page) and we flush all messages when the
first delegate is attached, even though there could be messages for different
nativeApp values which don't have a delegate attached yet.

We also erroneusly return a rejected promise to javascript when a message is queued up.

This patch addresses the above by:

  • Never rejecting a pending connection request, the connection request will be
    resolved when the delegate for the right nativeApp is attached.
  • Making the pending messages queue per-nativeApp and per-session.
  • Flushing pending messages when a session delegate is attached.
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/67f8778fe68b
Flush extension messages per-session and per-nativeApp. r=esawin

Backed out for geckoview failures

backout: https://hg.mozilla.org/integration/autoland/rev/bae851e5b1be84eb612ae7de6ea700711a84dec3

push: https://treeherder.mozilla.org/jobs?repo=autoland&searchStr=android%2C7.0%2Cx86-64%2Cwebrender%2Cdebug%2Ctest-android-em-7.0-x86_64-qr%2Fdebug-geckoview-junit-fis-e10s%2Cgv-junit-fis&revision=67f8778fe68b0d70893cf84d94633c4a69149237

failure log: https://treeherder.mozilla.org/logviewer?job_id=321497766&repo=autoland&lineNumber=14687

[task 2020-11-11T21:40:12.643Z] 21:40:12 INFO - TEST-START | org.mozilla.geckoview.test.WebExtensionTest.testRestoringExtensionPagePreservesMessages
[task 2020-11-11T21:40:42.981Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: numtests=766
[task 2020-11-11T21:40:42.981Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stream=
[task 2020-11-11T21:40:42.981Z] 21:40:42 INFO - org.mozilla.geckoview.test | Error in testRestoringExtensionPagePreservesMessages(org.mozilla.geckoview.test.WebExtensionTest):
[task 2020-11-11T21:40:42.982Z] 21:40:42 INFO - org.mozilla.geckoview.test | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2020-11-11T21:40:42.982Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutRunnable.run(UiThreadUtils.java:52)
[task 2020-11-11T21:40:42.982Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
[task 2020-11-11T21:40:42.982Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2020-11-11T21:40:42.982Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils.waitForCondition(UiThreadUtils.java:155)
[task 2020-11-11T21:40:42.982Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1587)
[task 2020-11-11T21:40:42.983Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1529)
[task 2020-11-11T21:40:42.983Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1482)
[task 2020-11-11T21:40:42.983Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.WebExtensionTest.testRestoringExtensionPagePreservesMessages(WebExtensionTest.kt:838)
[task 2020-11-11T21:40:42.983Z] 21:40:42 INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
[task 2020-11-11T21:40:42.983Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2020-11-11T21:40:42.983Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2020-11-11T21:40:42.984Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2020-11-11T21:40:42.984Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2020-11-11T21:40:42.984Z] 21:40:42 INFO - org.mozilla.geckoview.test | at androidx.test.internal.runner.junit4.statement.RunBefores.evaluate(RunBefores.java:80)
[task 2020-11-11T21:40:42.984Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$GeckoSessionTestRule$2(GeckoSessionTestRule.java:1313)
[task 2020-11-11T21:40:42.984Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.-$$Lambda$GeckoSessionTestRule$2$sIbRNaZJgAu-QrUVWSGD8JbPSWM.run(lambda)
[task 2020-11-11T21:40:42.984Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Looper.loop(Looper.java:154)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.app.ActivityThread.main(ActivityThread.java:6077)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
[task 2020-11-11T21:40:42.985Z] 21:40:42 INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test |
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: id=AndroidJUnitRunner
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: test=testRestoringExtensionPagePreservesMessages
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: class=org.mozilla.geckoview.test.WebExtensionTest
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: stack=org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutRunnable.run(UiThreadUtils.java:52)
[task 2020-11-11T21:40:42.986Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.util.UiThreadUtils.waitForCondition(UiThreadUtils.java:155)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1587)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1529)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule.waitUntilCalled(GeckoSessionTestRule.java:1482)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.WebExtensionTest.testRestoringExtensionPagePreservesMessages(WebExtensionTest.kt:838)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
[task 2020-11-11T21:40:42.987Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at androidx.test.internal.runner.junit4.statement.RunBefores.evaluate(RunBefores.java:80)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.GeckoSessionTestRule$2.lambda$evaluate$0$GeckoSessionTestRule$2(GeckoSessionTestRule.java:1313)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at org.mozilla.geckoview.test.rule.-$$Lambda$GeckoSessionTestRule$2$sIbRNaZJgAu-QrUVWSGD8JbPSWM.run(lambda)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.app.Instrumentation$SyncRunnable.run(Instrumentation.java:1950)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.handleCallback(Handler.java:751)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Handler.dispatchMessage(Handler.java:95)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.os.Looper.loop(Looper.java:154)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at android.app.ActivityThread.main(ActivityThread.java:6077)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at java.lang.reflect.Method.invoke(Native Method)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:866)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test | at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:756)
[task 2020-11-11T21:40:42.988Z] 21:40:42 INFO - org.mozilla.geckoview.test |
[task 2020-11-11T21:40:42.989Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS: current=704
[task 2020-11-11T21:40:42.989Z] 21:40:42 INFO - org.mozilla.geckoview.test | INSTRUMENTATION_STATUS_CODE: -2
[task 2020-11-11T21:40:42.989Z] 21:40:42 WARNING - TEST-UNEXPECTED-FAIL | org.mozilla.geckoview.test.WebExtensionTest.testRestoringExtensionPagePreservesMessages | org.mozilla.geckoview.test.util.UiThreadUtils$TimeoutException: Timed out after 30000ms
[task 2020-11-11T21:40:42.989Z] 21:40:42 INFO - TEST-INFO took 30345ms

Flags: needinfo?(agi)
Pushed by asferro@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/1778b134434a
Flush extension messages per-session and per-nativeApp. r=esawin

https://hg.mozilla.org/mozilla-central/rev/1778b134434a

Status: NEW → RESOLVED
Closed: 4 years ago
status-firefox84: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

Thanks!

Flags: needinfo?(agi)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: