-
Notifications
You must be signed in to change notification settings - Fork 218
Closes #2267 AC WebExtensions + WebCompat + FxAWebChannels #3498
Closes #2267 AC WebExtensions + WebCompat + FxAWebChannels #3498
Conversation
@pocmo As you were involved with the |
2a5ce50
to
2df98c8
Compare
Please rebase your patch with master first. I was helping you to rebase it in local, but you were working on your own remote, so I can't help you give a commit. After my investigation, your patch will skip calling |
If we move back
to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is a workable version to fix unit testing failures. (https://github.com/MozillaReality/FirefoxReality/tree/keianhzo_v11_fxa_webchannel_1)
There are two things I did. The first is rewriting GleanMetricService with Kotlin. The other is moving TelemetryWrapper.init(this); GleanMetricsService.INSTANCE.init(this);
back to onCreate()
.
In addition, I need to give a default PingUploader to the Config
val config = Configuration(
channel = BuildConfig.BUILD_TYPE,
httpClient = ConceptFetchHttpUploader(lazy { HttpURLConnectionClient() }))
Your version will crash in GeckoEngine somehow. You can try to still use GleanMetricService.java
first with proper way to initialize Configuration
. If we still need to switch to *.kt because their API constraint. I am going to give a PR to land my code.
Btw, we are better not to use the default client uploader. We should use GeckoView client uploader instead. I can work on it at another PR. |
2df98c8
to
5a145fe
Compare
@daoshengmu I've fixed the rebase issues.
This PR already uses the The main difference between this PR and master is that it uses the That seems to break Gecko somehow as nothing is renderer anymore and it also breaks the tests with:
As it seems that the The weird thing is that the GeckoRuntime docs states: |
app/src/common/shared/org/mozilla/vrbrowser/browser/components/GeckoEngineSession.kt
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/adapter/ComponentsAdapter.kt
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/components/GeckoEngineSession.kt
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/adapter/ComponentsAdapter.kt
Show resolved
Hide resolved
ecd15b0
to
6b99664
Compare
@daoshengmu I've fixed the GV issue but I still can see issues with test, especially thee issue with the crashservice process not being correctly determined from the manifest in test. I'll have look into that but if you have any ideas, they are welcomed. |
app/src/common/shared/org/mozilla/vrbrowser/browser/components/GeckoWebExtensionRuntime.kt
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/SettingsStore.java
Outdated
Show resolved
Hide resolved
You can run So far, I notice there is no one instances |
6b99664
to
a8df13f
Compare
@daoshengmu I have fixed the Telemetry tests. The GV team recommended to move the GeckoRuntime startup code to the Activity onCreate so I moved all the initialization there and initialized the Telemetry services for tests in a setup method. Seems to be working fine but I needed to add a flag to avoid the legacy LegacyTelemetry initialization. |
0748a7f
to
872472e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just do changes in tests instead of adding a flag in GleanMetricsServiceTest
.
app/src/test/java/org/mozilla/vrbrowser/GleanMetricsServiceTest.kt
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/telemetry/GleanMetricsService.java
Outdated
Show resolved
Hide resolved
app/src/test/java/org/mozilla/vrbrowser/GleanMetricsServiceTest.kt
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/VRBrowserApplication.java
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/Services.kt
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/telemetry/GleanMetricsService.java
Outdated
Show resolved
Hide resolved
8cef319
to
ebdf8b3
Compare
app/src/common/shared/org/mozilla/vrbrowser/browser/engine/Session.java
Outdated
Show resolved
Hide resolved
app/src/common/shared/org/mozilla/vrbrowser/browser/engine/SessionStore.java
Show resolved
Hide resolved
ebdf8b3
to
4e55050
Compare
Closes #2267 Closes #3339 This PR adds support for the following:
ComponentsAdapter
as a bridge between ourSessionStore
and AC'sBrowserStore
which is required by the AC WebExtensions/Features lifecycle.WebExtensionsRuntime
.WebCompat
extension.SessionStore
is now theWebExtensionDelegate
SessionStore
as the single point of communications with theBrowserStore
.There are some Glean related updates, mainly initialization changes and lint errors in .md files, could you please @daoshengmu verify that everything is working as expected?
@bluemarvin @MortimerGoro I've have replaced the appcompat aar for the maven version so it stays aligned with the rest of the androidx components. I couldn't find any speech related classes in that module.
QA:
This should affect mainly to the WebExtensions. Things that we should keep an eye on: