Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC Unified Chrome Management #3311

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
@@ -7,6 +7,7 @@ plugins {
alias libs.plugins.compose.compiler
alias libs.plugins.dependency.analysis
alias libs.plugins.kotlin.android
alias libs.plugins.androidx.navigation.safeargs
}

apply from: "$project.rootDir/automation/gradle/versionCode.gradle"
@@ -251,6 +252,8 @@ dependencies {
implementation libs.androidx.recyclerview
implementation libs.androidx.swiperefreshlayout
implementation libs.androidx.work.runtime.ktx
implementation libs.androidx.navigation.fragment
implementation libs.androidx.navigation.ui

implementation platform(libs.androidx.compose.bom)
implementation libs.androidx.compose.foundation
Original file line number Diff line number Diff line change
@@ -25,6 +25,7 @@ import mozilla.components.support.webextensions.WebExtensionPopupObserver
import org.mozilla.reference.browser.addons.WebExtensionActionPopupActivity
import org.mozilla.reference.browser.browser.BrowserFragment
import org.mozilla.reference.browser.browser.CrashIntegration
import org.mozilla.reference.browser.browser.MainContainerFragment
import org.mozilla.reference.browser.ext.components
import org.mozilla.reference.browser.ext.isCrashReportActive

@@ -33,6 +34,8 @@ import org.mozilla.reference.browser.ext.isCrashReportActive
*/
open class BrowserActivity : AppCompatActivity() {

private val logger = Logger("BrowserActivity")

private lateinit var crashIntegration: CrashIntegration

private val sessionId: String?
@@ -46,7 +49,7 @@ open class BrowserActivity : AppCompatActivity() {
* Returns a new instance of [BrowserFragment] to display.
*/
open fun createBrowserFragment(sessionId: String?): Fragment =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having BrowserActivity call createBrowserFragment which would create MainContainerFragment seems forced.
RB uses one activity per feature so if the feature now is "Main.."
Maybe this should be renames to MainActivity & createMainFragment.
Speaking of which, why do we need a fragment parent for home & browser and not just use this Activity?

BrowserFragment.create(sessionId)
MainContainerFragment.create(sessionId)

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
@@ -75,7 +78,9 @@ open class BrowserActivity : AppCompatActivity() {
@Suppress("MissingSuperCall", "OVERRIDE_DEPRECATION")
override fun onBackPressed() {
supportFragmentManager.fragments.forEach {
logger.debug("onBackPressed fragment: $it")
if (it is UserInteractionHandler && it.onBackPressed()) {
logger.debug("onBackPressed UserInteractionHandler: $it")
return
}
}
Original file line number Diff line number Diff line change
@@ -14,14 +14,11 @@ import android.view.ViewGroup
import androidx.activity.result.ActivityResultLauncher
import androidx.activity.result.contract.ActivityResultContracts
import androidx.annotation.CallSuper
import androidx.compose.ui.platform.ComposeView
import androidx.coordinatorlayout.widget.CoordinatorLayout
import androidx.fragment.app.Fragment
import androidx.preference.PreferenceManager
import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
import mozilla.components.browser.state.selector.selectedTab
import mozilla.components.browser.toolbar.BrowserToolbar
import mozilla.components.compose.browser.toolbar.BrowserToolbar
import mozilla.components.concept.engine.EngineView
import mozilla.components.feature.app.links.AppLinksFeature
import mozilla.components.feature.downloads.DownloadsFeature
@@ -45,7 +42,6 @@ import mozilla.components.support.base.log.logger.Logger
import mozilla.components.support.ktx.android.view.enterImmersiveMode
import mozilla.components.support.ktx.android.view.exitImmersiveMode
import mozilla.components.ui.widgets.behavior.EngineViewClippingBehavior
import mozilla.components.ui.widgets.behavior.EngineViewScrollingBehavior
import org.mozilla.reference.browser.BuildConfig
import org.mozilla.reference.browser.R
import org.mozilla.reference.browser.addons.WebExtensionPromptFeature
@@ -55,7 +51,6 @@ import org.mozilla.reference.browser.ext.requireComponents
import org.mozilla.reference.browser.pip.PictureInPictureIntegration
import org.mozilla.reference.browser.tabs.LastTabFeature
import mozilla.components.ui.widgets.behavior.ToolbarPosition as MozacEngineBehaviorToolbarPosition
import mozilla.components.ui.widgets.behavior.ViewPosition as MozacToolbarBehaviorToolbarPosition

/**
* Base fragment extended by [BrowserFragment] and [ExternalAppBrowserFragment].
@@ -64,7 +59,6 @@ import mozilla.components.ui.widgets.behavior.ViewPosition as MozacToolbarBehavi
*/
abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, ActivityResultHandler {
private val sessionFeature = ViewBoundFeatureWrapper<SessionFeature>()
private val toolbarIntegration = ViewBoundFeatureWrapper<ToolbarIntegration>()
private val contextMenuIntegration = ViewBoundFeatureWrapper<ContextMenuIntegration>()
private val downloadsFeature = ViewBoundFeatureWrapper<DownloadsFeature>()
private val shareDownloadsFeature = ViewBoundFeatureWrapper<ShareDownloadFeature>()
@@ -84,18 +78,15 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit

private val engineView: EngineView
get() = requireView().findViewById<View>(R.id.engineView) as EngineView
private val toolbar: BrowserToolbar
get() = requireView().findViewById(R.id.toolbar)
private val findInPageBar: FindInPageBar
protected val findInPageBar: FindInPageBar
get() = requireView().findViewById(R.id.findInPageBar)
private val swipeRefresh: SwipeRefreshLayout
get() = requireView().findViewById(R.id.swipeRefresh)

private val backButtonHandler: List<ViewBoundFeatureWrapper<*>> = listOf(
fullScreenFeature,
findInPageIntegration,
toolbarIntegration,

// toolbarIntegration,
sessionFeature,
lastTabFeature,
)
@@ -114,6 +105,8 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
private lateinit var requestSitePermissionsLauncher: ActivityResultLauncher<Array<String>>
private lateinit var requestPromptsPermissionsLauncher: ActivityResultLauncher<Array<String>>

private val logger = Logger("BaseBrowserFragment")

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
requestDownloadPermissionsLauncher =
@@ -153,16 +146,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
}
}

final override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?,
): View {
return inflater.inflate(R.layout.fragment_browser, container, false)
}

abstract val shouldUseComposeUI: Boolean

@CallSuper
@Suppress("LongMethod")
override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
@@ -180,28 +163,6 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
view = view,
)

(toolbar.layoutParams as? CoordinatorLayout.LayoutParams)?.apply {
behavior = EngineViewScrollingBehavior(
view.context,
null,
MozacToolbarBehaviorToolbarPosition.BOTTOM,
)
}
toolbarIntegration.set(
feature = ToolbarIntegration(
requireContext(),
toolbar,
requireComponents.core.historyStorage,
requireComponents.core.store,
requireComponents.useCases.sessionUseCases,
requireComponents.useCases.tabsUseCases,
requireComponents.useCases.webAppUseCases,
sessionId,
),
owner = this,
view = view,
)

contextMenuIntegration.set(
feature = ContextMenuIntegration(
requireContext(),
@@ -357,7 +318,7 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
context,
null,
swipeRefresh,
toolbar.height,
resources.getDimensionPixelSize(R.dimen.browser_toolbar_height),
MozacEngineBehaviorToolbarPosition.BOTTOM,
)
}
@@ -402,28 +363,21 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
view = view,
)
}

val composeView = view.findViewById<ComposeView>(R.id.compose_view)
if (shouldUseComposeUI) {
composeView.visibility = View.VISIBLE
composeView.setContent { BrowserToolbar() }

val params = swipeRefresh.layoutParams as CoordinatorLayout.LayoutParams
params.topMargin = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height)
swipeRefresh.layoutParams = params
}
}

private fun fullScreenChanged(enabled: Boolean) {
if (enabled) {
activity?.enterImmersiveMode()
toolbar.visibility = View.GONE
engineView.setDynamicToolbarMaxHeight(0)
} else {
activity?.exitImmersiveMode()
toolbar.visibility = View.VISIBLE
engineView.setDynamicToolbarMaxHeight(resources.getDimensionPixelSize(R.dimen.browser_toolbar_height))
}
parentFragment?.parentFragmentManager
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Communicating between fragments or with the parent should normally be done through a parent ViewModel or in our case a Store.
Or as it happens in many cases and if we know that our parent is showing a toolbar we could just access it directly.

Curious why going with the fragment result instead.

?.setFragmentResult(
BROWSER_TO_MAIN_FRAGMENT_RESULT_KEY,
Bundle().apply { putBoolean(FULL_SCREEN_MODE_CHANGED, enabled) }
)
}

private fun viewportFitChanged(viewportFit: Int) {
@@ -434,7 +388,10 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit

@CallSuper
override fun onBackPressed(): Boolean {
return backButtonHandler.any { it.onBackPressed() }
logger.info("onBackPressed")
return backButtonHandler.any { it.onBackPressed() }.also {
logger.info("Was it handled by back button handlers? $it")
}
}

final override fun onHomePressed(): Boolean {
@@ -452,16 +409,14 @@ abstract class BaseBrowserFragment : Fragment(), UserInteractionHandler, Activit
}

companion object {
private const val SESSION_ID = "session_id"

@JvmStatic
protected fun Bundle.putSessionId(sessionId: String?) {
putString(SESSION_ID, sessionId)
}
}

override fun onActivityResult(requestCode: Int, data: Intent?, resultCode: Int): Boolean {
Logger.info(
logger.info(
"Fragment onActivityResult received with " +
"requestCode: $requestCode, resultCode: $resultCode, data: $data",
)
Loading