From f7542f3e334e646b1b8790373d687c4c3b56476d Mon Sep 17 00:00:00 2001 From: AndiAJ Date: Wed, 13 Mar 2024 14:17:28 +0200 Subject: [PATCH 1/6] Bug 1885123 - Add logs to DataGenerationHelper --- .../fenix/helpers/DataGenerationHelper.kt | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/DataGenerationHelper.kt b/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/DataGenerationHelper.kt index d82affa717ff..44e46b135507 100644 --- a/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/DataGenerationHelper.kt +++ b/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/DataGenerationHelper.kt @@ -13,6 +13,7 @@ import android.graphics.Bitmap import android.graphics.Canvas import android.graphics.Color import android.net.Uri +import android.util.Log import androidx.browser.customtabs.CustomTabsIntent import androidx.test.platform.app.InstrumentationRegistry import androidx.test.uiautomator.UiSelector @@ -20,6 +21,7 @@ import mozilla.components.browser.state.search.SearchEngine import mozilla.components.browser.state.state.availableSearchEngines import org.junit.Assert import org.mozilla.fenix.ext.components +import org.mozilla.fenix.helpers.Constants.TAG import org.mozilla.fenix.helpers.TestHelper.mDevice import org.mozilla.fenix.utils.IntentUtils import java.time.LocalDate @@ -33,6 +35,7 @@ object DataGenerationHelper { customMenuItemLabel: String = "", customActionButtonDescription: String = "", ): Intent { + Log.i(TAG, "createCustomTabIntent: Trying to create custom tab intent with url: $pageUrl") val appContext = InstrumentationRegistry.getInstrumentation() .targetContext .applicationContext @@ -48,33 +51,44 @@ object DataGenerationHelper { ) .build() customTabsIntent.intent.data = Uri.parse(pageUrl) + Log.i(TAG, "createCustomTabIntent: Created custom tab intent with url: $pageUrl") return customTabsIntent.intent } private fun createTestBitmap(): Bitmap { + Log.i(TAG, "createTestBitmap: Trying to create a test bitmap") val bitmap = Bitmap.createBitmap(100, 100, Bitmap.Config.ARGB_8888) val canvas = Canvas(bitmap) canvas.drawColor(Color.GREEN) + Log.i(TAG, "createTestBitmap: Created a test bitmap") return bitmap } fun getStringResource(id: Int, argument: String = TestHelper.appName) = TestHelper.appContext.resources.getString(id, argument) private val charPool: List = ('a'..'z') + ('A'..'Z') + ('0'..'9') - fun generateRandomString(stringLength: Int) = - (1..stringLength) - .map { kotlin.random.Random.nextInt(0, charPool.size) } - .map(charPool::get) - .joinToString("") + fun generateRandomString(stringLength: Int): String { + Log.i(TAG, "generateRandomString: Trying to generate a random string with $stringLength characters") + val randomString = + (1..stringLength) + .map { kotlin.random.Random.nextInt(0, charPool.size) } + .map(charPool::get) + .joinToString("") + Log.i(TAG, "generateRandomString: Generated random string: $randomString") + + return randomString + } /** * Creates clipboard data. */ fun setTextToClipBoard(context: Context, message: String) { + Log.i(TAG, "setTextToClipBoard: Trying to set clipboard text to: $message") val clipBoard = context.getSystemService(Context.CLIPBOARD_SERVICE) as ClipboardManager val clipData = ClipData.newPlainText("label", message) clipBoard.setPrimaryClip(clipData) + Log.i(TAG, "setTextToClipBoard: Clipboard text was set to: $message") } /** @@ -88,6 +102,7 @@ object DataGenerationHelper { * @return A string representing the current date and time in the specified format. */ fun getSponsoredFxSuggestPlaceHolder(): String { + Log.i(TAG, "getSponsoredFxSuggestPlaceHolder: Trying to get the sponsored search suggestion placeholder") val currentDate = LocalDate.now() val currentTime = LocalTime.now() @@ -96,6 +111,8 @@ object DataGenerationHelper { val currentYear = currentDate.year.toString() val currentHour = currentTime.hour.toString().padStart(2, '0') + Log.i(TAG, "getSponsoredFxSuggestPlaceHolder: Got: ${currentYear + currentMonth + currentDay + currentHour} as the sponsored search suggestion placeholder") + return currentYear + currentMonth + currentDay + currentHour } @@ -103,6 +120,7 @@ object DataGenerationHelper { * Returns sponsored shortcut title based on the index. */ fun getSponsoredShortcutTitle(position: Int): String { + Log.i(TAG, "getSponsoredShortcutTitle: Trying to get the title of the sponsored shortcut at position: ${position - 1}") val sponsoredShortcut = mDevice.findObject( UiSelector() .resourceId("${TestHelper.packageName}:id/top_site_item") @@ -111,7 +129,7 @@ object DataGenerationHelper { UiSelector() .resourceId("${TestHelper.packageName}:id/top_site_title"), ).text - + Log.i(TAG, "getSponsoredShortcutTitle: The sponsored shortcut at position: ${position - 1} has title: $sponsoredShortcut") return sponsoredShortcut } @@ -120,8 +138,10 @@ object DataGenerationHelper { * For en-us it will return the 6 engines selected by default: Google, Bing, DuckDuckGo, Amazon, Ebay, Wikipedia. */ fun getRegionSearchEnginesList(): List { + Log.i(TAG, "getRegionSearchEnginesList: Trying to get the search engines based on the region of the user") val searchEnginesList = appContext.components.core.store.state.search.regionSearchEngines - Assert.assertTrue("Search engines list returned nothing", searchEnginesList.isNotEmpty()) + Assert.assertTrue("$TAG: Search engines list returned nothing", searchEnginesList.isNotEmpty()) + Log.i(TAG, "getRegionSearchEnginesList: Got $searchEnginesList based on the region of the user") return searchEnginesList } @@ -130,8 +150,10 @@ object DataGenerationHelper { * For en-us it will return the 2 engines: Reddit, Youtube. */ fun getAvailableSearchEngines(): List { + Log.i(TAG, "getAvailableSearchEngines: Trying to get the alternative search engines based on the region of the user") val searchEnginesList = TestHelper.appContext.components.core.store.state.search.availableSearchEngines - Assert.assertTrue("Search engines list returned nothing", searchEnginesList.isNotEmpty()) + Assert.assertTrue("$TAG: Search engines list returned nothing", searchEnginesList.isNotEmpty()) + Log.i(TAG, "getAvailableSearchEngines: Got $searchEnginesList based on the region of the user") return searchEnginesList } } From 15c6eb2c4a22e68b65f982b59ea83c8e7a6886ef Mon Sep 17 00:00:00 2001 From: Rahul Sainani Date: Wed, 13 Mar 2024 09:58:16 +0100 Subject: [PATCH 2/6] Revert "Bug 1867717 - Add DownloadsFeature to AddonPopupBaseFragment." This reverts commit cc65693e4f8811906f344d0e902af0bccbb7edbf. --- .../addons/AddonInternalSettingsFragment.kt | 18 +- .../fenix/addons/AddonPopupBaseFragment.kt | 124 +------------ .../addons/WebExtensionActionPopupFragment.kt | 19 +- .../fenix/browser/BaseBrowserFragment.kt | 86 ++++----- .../fenix/browser/DownloadDialogUtils.kt | 62 ------- .../mozilla/fenix/browser/DownloadUtils.kt | 53 ++++++ .../mozilla/fenix/components/Components.kt | 11 -- .../fenix/components/DownloadStyling.kt | 36 ---- .../fragment_add_on_internal_settings.xml | 14 -- .../addons/AddonPopupBaseFragmentTest.kt | 53 ------ .../fenix/browser/BaseBrowserFragmentTest.kt | 73 +++++++- .../fenix/browser/DownloadDialogUtilsTest.kt | 173 ------------------ 12 files changed, 165 insertions(+), 557 deletions(-) delete mode 100644 fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadDialogUtils.kt create mode 100644 fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadUtils.kt delete mode 100644 fenix/app/src/main/java/org/mozilla/fenix/components/DownloadStyling.kt delete mode 100644 fenix/app/src/test/java/org/mozilla/fenix/addons/AddonPopupBaseFragmentTest.kt delete mode 100644 fenix/app/src/test/java/org/mozilla/fenix/browser/DownloadDialogUtilsTest.kt diff --git a/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonInternalSettingsFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonInternalSettingsFragment.kt index 7155c90ee34f..8c08a95fd778 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonInternalSettingsFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonInternalSettingsFragment.kt @@ -12,7 +12,6 @@ import androidx.navigation.fragment.findNavController import androidx.navigation.fragment.navArgs import mozilla.components.feature.addons.ui.translateName import org.mozilla.fenix.R -import org.mozilla.fenix.databinding.DownloadDialogLayoutBinding import org.mozilla.fenix.databinding.FragmentAddOnInternalSettingsBinding import org.mozilla.fenix.ext.showToolbar @@ -22,8 +21,6 @@ import org.mozilla.fenix.ext.showToolbar class AddonInternalSettingsFragment : AddonPopupBaseFragment() { private val args by navArgs() - private var _binding: FragmentAddOnInternalSettingsBinding? = null - internal val binding get() = _binding!! override fun onCreateView( inflater: LayoutInflater, @@ -34,17 +31,9 @@ class AddonInternalSettingsFragment : AddonPopupBaseFragment() { return inflater.inflate(R.layout.fragment_add_on_internal_settings, container, false) } - override fun getSnackBarContainer(): ViewGroup { - return binding.dynamicSnackbarContainer - } - - override fun getDownloadDialogLayoutBinding(): DownloadDialogLayoutBinding { - return binding.viewDynamicDownloadDialog - } - override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - _binding = FragmentAddOnInternalSettingsBinding.bind(view) + val binding = FragmentAddOnInternalSettingsBinding.bind(view) args.addon.installedState?.optionsPageUrl?.let { engineSession?.let { engineSession -> binding.addonSettingsEngineView.render(engineSession) @@ -59,9 +48,4 @@ class AddonInternalSettingsFragment : AddonPopupBaseFragment() { showToolbar(title = args.addon.translateName(it)) } } - - override fun onDestroyView() { - super.onDestroyView() - _binding = null - } } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt index 05eeb740b6eb..b1c54ee1f083 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/addons/AddonPopupBaseFragment.kt @@ -5,33 +5,21 @@ package org.mozilla.fenix.addons import android.os.Bundle -import android.os.Environment import android.view.View -import android.view.ViewGroup -import androidx.annotation.VisibleForTesting import androidx.fragment.app.Fragment import androidx.navigation.fragment.findNavController -import com.google.android.material.snackbar.Snackbar import mozilla.components.browser.state.action.ContentAction import mozilla.components.browser.state.action.CustomTabListAction import mozilla.components.browser.state.state.CustomTabSessionState import mozilla.components.browser.state.state.EngineState import mozilla.components.browser.state.state.SessionState -import mozilla.components.browser.state.state.content.DownloadState import mozilla.components.browser.state.state.createCustomTab import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.prompt.PromptRequest import mozilla.components.concept.engine.window.WindowRequest -import mozilla.components.concept.fetch.Response -import mozilla.components.feature.downloads.DownloadsFeature import mozilla.components.feature.prompts.PromptFeature import mozilla.components.support.base.feature.UserInteractionHandler import mozilla.components.support.base.feature.ViewBoundFeatureWrapper -import org.mozilla.fenix.browser.DownloadDialogUtils -import org.mozilla.fenix.components.DownloadStyling -import org.mozilla.fenix.components.FenixSnackbar -import org.mozilla.fenix.databinding.DownloadDialogLayoutBinding -import org.mozilla.fenix.downloads.DynamicDownloadDialog import org.mozilla.fenix.ext.requireComponents /** @@ -40,22 +28,19 @@ import org.mozilla.fenix.ext.requireComponents */ abstract class AddonPopupBaseFragment : Fragment(), EngineSession.Observer, UserInteractionHandler { private val promptsFeature = ViewBoundFeatureWrapper() - private val downloadsFeature = ViewBoundFeatureWrapper() - internal var session: SessionState? = null + protected var session: SessionState? = null protected var engineSession: EngineSession? = null private var canGoBack: Boolean = false @Suppress("DEPRECATION") // https://github.com/mozilla-mobile/fenix/issues/19920 override fun onViewCreated(view: View, savedInstanceState: Bundle?) { - val store = requireComponents.core.store - val safeContext = requireContext() session?.let { promptsFeature.set( feature = PromptFeature( fragment = this, - store = store, + store = requireComponents.core.store, customTabId = it.id, fragmentManager = parentFragmentManager, fileUploadsDirCleaner = requireComponents.core.fileUploadsDirCleaner, @@ -67,52 +52,6 @@ abstract class AddonPopupBaseFragment : Fragment(), EngineSession.Observer, User owner = this, view = view, ) - - downloadsFeature.set( - DownloadsFeature( - safeContext, - store = store, - useCases = requireComponents.useCases.downloadUseCases, - fragmentManager = parentFragmentManager, - tabId = it.id, - downloadManager = requireComponents.downloadManager, - promptsStyling = DownloadStyling.createPrompt(safeContext), - onNeedToRequestPermissions = { permissions -> - requestPermissions(permissions, REQUEST_CODE_DOWNLOAD_PERMISSIONS) - }, - ), - owner = this, - view = view, - ) - - downloadsFeature.get()?.onDownloadStopped = { downloadState, _, downloadJobStatus -> - val onCannotOpenFile: (DownloadState) -> Unit = { - FenixSnackbar.make( - view = getSnackBarContainer(), - duration = Snackbar.LENGTH_SHORT, - isDisplayedWithBrowserToolbar = true, - ).setText(DynamicDownloadDialog.getCannotOpenFileErrorMessage(requireContext(), it)) - .show() - } - DownloadDialogUtils.handleOnDownloadFinished( - context = requireContext(), - downloadState = downloadState, - downloadJobStatus = downloadJobStatus, - currentTab = it, - onCannotOpenFile = onCannotOpenFile, - onFinishedDialogShown = { - DynamicDownloadDialog( - context = requireContext(), - downloadState = downloadState, - didFail = downloadJobStatus == DownloadState.Status.FAILED, - tryAgain = downloadsFeature.get()!!::tryAgain, - onCannotOpenFile = onCannotOpenFile, - binding = getDownloadDialogLayoutBinding(), - toolbarHeight = 0, - ) {}.show() - }, - ) - } } } @@ -134,44 +73,6 @@ abstract class AddonPopupBaseFragment : Fragment(), EngineSession.Observer, User engineSession?.unregister(this) } - override fun onExternalResource( - url: String, - fileName: String?, - contentLength: Long?, - contentType: String?, - cookie: String?, - userAgent: String?, - isPrivate: Boolean, - skipConfirmation: Boolean, - openInApp: Boolean, - response: Response?, - ) { - session?.let { session -> - val fileSize = if (contentLength != null && contentLength < 0) null else contentLength - val download = DownloadState( - url, - fileName, - contentType, - fileSize, - 0, - DownloadState.Status.INITIATED, - userAgent, - Environment.DIRECTORY_DOWNLOADS, - private = isPrivate, - skipConfirmation = skipConfirmation, - openInApp = openInApp, - response = response, - ) - - provideBrowserStore().dispatch( - ContentAction.UpdateDownloadAction( - session.id, - download, - ), - ) - } - } - override fun onPromptRequest(promptRequest: PromptRequest) { session?.let { session -> requireComponents.core.store.dispatch( @@ -213,34 +114,17 @@ abstract class AddonPopupBaseFragment : Fragment(), EngineSession.Observer, User requireComponents.core.store.dispatch(CustomTabListAction.AddCustomTabAction(session as CustomTabSessionState)) } - @VisibleForTesting - internal fun provideBrowserStore() = requireComponents.core.store - final override fun onRequestPermissionsResult( requestCode: Int, permissions: Array, grantResults: IntArray, ) { - val feature = when (requestCode) { - REQUEST_CODE_PROMPT_PERMISSIONS -> promptsFeature.get() - REQUEST_CODE_DOWNLOAD_PERMISSIONS -> downloadsFeature.get() - else -> null + when (requestCode) { + REQUEST_CODE_PROMPT_PERMISSIONS -> promptsFeature.get()?.onPermissionsResult(permissions, grantResults) } - feature?.onPermissionsResult(permissions, grantResults) } - /** - * Returns a [ViewGroup] where a SnackBar message should be anchored. - */ - abstract fun getSnackBarContainer(): ViewGroup - - /** - * Returns a [DownloadDialogLayoutBinding] to access the download dialog items. - */ - abstract fun getDownloadDialogLayoutBinding(): DownloadDialogLayoutBinding - companion object { private const val REQUEST_CODE_PROMPT_PERMISSIONS = 1 - private const val REQUEST_CODE_DOWNLOAD_PERMISSIONS = 2 } } diff --git a/fenix/app/src/main/java/org/mozilla/fenix/addons/WebExtensionActionPopupFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/addons/WebExtensionActionPopupFragment.kt index 12fa3597a94a..95b2246c5cbe 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/addons/WebExtensionActionPopupFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/addons/WebExtensionActionPopupFragment.kt @@ -15,7 +15,6 @@ import mozilla.components.concept.engine.EngineSession import mozilla.components.concept.engine.EngineView import mozilla.components.lib.state.ext.consumeFrom import org.mozilla.fenix.R -import org.mozilla.fenix.databinding.DownloadDialogLayoutBinding import org.mozilla.fenix.databinding.FragmentAddOnInternalSettingsBinding import org.mozilla.fenix.ext.requireComponents import org.mozilla.fenix.ext.showToolbar @@ -34,9 +33,6 @@ class WebExtensionActionPopupFragment : AddonPopupBaseFragment(), EngineSession. safeArguments.putBoolean("isSessionConsumed", value) } - private var _binding: FragmentAddOnInternalSettingsBinding? = null - internal val binding get() = _binding!! - override fun onCreateView( inflater: LayoutInflater, container: ViewGroup?, @@ -50,14 +46,6 @@ class WebExtensionActionPopupFragment : AddonPopupBaseFragment(), EngineSession. return inflater.inflate(R.layout.fragment_add_on_internal_settings, container, false) } - override fun getSnackBarContainer(): ViewGroup { - return binding.dynamicSnackbarContainer - } - - override fun getDownloadDialogLayoutBinding(): DownloadDialogLayoutBinding { - return binding.viewDynamicDownloadDialog - } - override fun onResume() { super.onResume() val title = args.webExtensionTitle ?: args.webExtensionId @@ -67,7 +55,7 @@ class WebExtensionActionPopupFragment : AddonPopupBaseFragment(), EngineSession. override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) - _binding = FragmentAddOnInternalSettingsBinding.bind(view) + val binding = FragmentAddOnInternalSettingsBinding.bind(view) val session = engineSession // If we have the session, render it otherwise consume it from the store. @@ -94,11 +82,6 @@ class WebExtensionActionPopupFragment : AddonPopupBaseFragment(), EngineSession. } } - override fun onDestroyView() { - super.onDestroyView() - _binding = null - } - private fun consumePopupSession() { coreComponents.store.dispatch( WebExtensionAction.UpdatePopupSessionAction(args.webExtensionId, popupSession = null), diff --git a/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt b/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt index 59999afaa939..6de923a5a27c 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/browser/BaseBrowserFragment.kt @@ -13,6 +13,7 @@ import android.os.Build import android.os.Bundle import android.provider.Settings import android.util.Log +import android.view.Gravity import android.view.LayoutInflater import android.view.View import android.view.ViewGroup @@ -72,6 +73,7 @@ import mozilla.components.feature.app.links.AppLinksFeature import mozilla.components.feature.contextmenu.ContextMenuCandidate import mozilla.components.feature.contextmenu.ContextMenuFeature import mozilla.components.feature.downloads.DownloadsFeature +import mozilla.components.feature.downloads.manager.FetchDownloadManager import mozilla.components.feature.downloads.temporary.CopyDownloadFeature import mozilla.components.feature.downloads.temporary.ShareDownloadFeature import mozilla.components.feature.intent.ext.EXTRA_SESSION_ID @@ -127,7 +129,6 @@ import org.mozilla.fenix.OnBackLongPressedListener import org.mozilla.fenix.R import org.mozilla.fenix.browser.browsingmode.BrowsingMode import org.mozilla.fenix.browser.readermode.DefaultReaderModeController -import org.mozilla.fenix.components.DownloadStyling import org.mozilla.fenix.components.FenixSnackbar import org.mozilla.fenix.components.FindInPageIntegration import org.mozilla.fenix.components.StoreProvider @@ -150,6 +151,7 @@ import org.mozilla.fenix.compose.Divider import org.mozilla.fenix.crashes.CrashContentIntegration import org.mozilla.fenix.customtabs.ExternalAppBrowserActivity import org.mozilla.fenix.databinding.FragmentBrowserBinding +import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.downloads.DynamicDownloadDialog import org.mozilla.fenix.downloads.FirstPartyDownloadDialog import org.mozilla.fenix.downloads.StartDownloadDialog @@ -639,14 +641,31 @@ abstract class BaseBrowserFragment : useCases = context.components.useCases.downloadUseCases, fragmentManager = childFragmentManager, tabId = customTabSessionId, - downloadManager = context.components.downloadManager, + downloadManager = FetchDownloadManager( + context.applicationContext, + store, + DownloadService::class, + notificationsDelegate = context.components.notificationsDelegate, + ), shouldForwardToThirdParties = { PreferenceManager.getDefaultSharedPreferences(context).getBoolean( context.getPreferenceKey(R.string.pref_key_external_download_manager), false, ) }, - promptsStyling = DownloadStyling.createPrompt(context), + promptsStyling = DownloadsFeature.PromptsStyling( + gravity = Gravity.BOTTOM, + shouldWidthMatchParent = true, + positiveButtonBackgroundColor = ThemeManager.resolveAttribute( + R.attr.accent, + context, + ), + positiveButtonTextColor = ThemeManager.resolveAttribute( + R.attr.textOnColorPrimary, + context, + ), + positiveButtonRadius = (resources.getDimensionPixelSize(R.dimen.tab_corner_radius)).toFloat(), + ), onNeedToRequestPermissions = { permissions -> requestPermissions(permissions, REQUEST_CODE_DOWNLOAD_PERMISSIONS) }, @@ -699,46 +718,7 @@ abstract class BaseBrowserFragment : ) downloadFeature.onDownloadStopped = { downloadState, _, downloadJobStatus -> - val onCannotOpenFile: (DownloadState) -> Unit = { - FenixSnackbar.make( - view = binding.dynamicSnackbarContainer, - duration = Snackbar.LENGTH_SHORT, - isDisplayedWithBrowserToolbar = true, - ).setText( - DynamicDownloadDialog.getCannotOpenFileErrorMessage( - context, - downloadState, - ), - ).show() - } - - DownloadDialogUtils.handleOnDownloadFinished( - context = requireContext(), - downloadState = downloadState, - downloadJobStatus = downloadJobStatus, - currentTab = getCurrentTab(), - onFinishedDialogShown = { - saveDownloadDialogState( - downloadState.sessionId, - downloadState, - downloadJobStatus, - ) - browserToolbarView.expand() - - DynamicDownloadDialog( - context = context, - downloadState = downloadState, - didFail = downloadJobStatus == DownloadState.Status.FAILED, - tryAgain = downloadFeature::tryAgain, - onCannotOpenFile = onCannotOpenFile, - binding = binding.viewDynamicDownloadDialog, - toolbarHeight = toolbarHeight, - ) { - sharedViewModel.downloadDialogState.remove(downloadState.sessionId) - }.show() - }, - onCannotOpenFile = onCannotOpenFile, - ) + handleOnDownloadFinished(downloadState, downloadJobStatus, downloadFeature::tryAgain) } resumeDownloadDialogState( @@ -1244,12 +1224,7 @@ abstract class BaseBrowserFragment : didFail = savedDownloadState.second, tryAgain = onTryAgain, onCannotOpenFile = { - FenixSnackbar.make( - view = binding.dynamicSnackbarContainer, - duration = Snackbar.LENGTH_SHORT, - isDisplayedWithBrowserToolbar = true, - ).setText(DynamicDownloadDialog.getCannotOpenFileErrorMessage(context, it)) - .show() + showCannotOpenFileError(binding.dynamicSnackbarContainer, context, it) }, binding = binding.viewDynamicDownloadDialog, toolbarHeight = toolbarHeight, @@ -1757,6 +1732,19 @@ abstract class BaseBrowserFragment : ) } + internal fun showCannotOpenFileError( + container: ViewGroup, + context: Context, + downloadState: DownloadState, + ) { + FenixSnackbar.make( + view = container, + duration = Snackbar.LENGTH_SHORT, + isDisplayedWithBrowserToolbar = true, + ).setText(DynamicDownloadDialog.getCannotOpenFileErrorMessage(context, downloadState)) + .show() + } + companion object { private const val KEY_CUSTOM_TAB_SESSION_ID = "custom_tab_session_id" private const val REQUEST_CODE_DOWNLOAD_PERMISSIONS = 1 diff --git a/fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadDialogUtils.kt b/fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadDialogUtils.kt deleted file mode 100644 index ee5926b5b052..000000000000 --- a/fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadDialogUtils.kt +++ /dev/null @@ -1,62 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.browser - -import android.content.Context -import androidx.annotation.VisibleForTesting -import mozilla.components.browser.state.state.SessionState -import mozilla.components.browser.state.state.content.DownloadState -import mozilla.components.browser.state.state.content.DownloadState.Status -import mozilla.components.feature.downloads.AbstractFetchDownloadService - -/** - * Utilities for handling download dialogs. - */ -object DownloadDialogUtils { - - internal fun handleOnDownloadFinished( - context: Context, - downloadState: DownloadState, - downloadJobStatus: Status, - currentTab: SessionState?, - onFinishedDialogShown: () -> Unit = {}, - onCannotOpenFile: (DownloadState) -> Unit, - ) { - // If the download is just paused, don't show any in-app notification - if (shouldShowCompletedDownloadDialog(downloadState, downloadJobStatus, currentTab)) { - if (downloadState.openInApp && downloadJobStatus == Status.COMPLETED) { - val fileWasOpened = openFile(context, downloadState) - if (!fileWasOpened) { - onCannotOpenFile(downloadState) - } - } else { - onFinishedDialogShown() - } - } - } - - @VisibleForTesting - internal var openFile: (Context, DownloadState) -> (Boolean) = { context, downloadState -> - AbstractFetchDownloadService.openFile( - applicationContext = context.applicationContext, - download = downloadState, - ) - } - - /** - * Indicates whether or not a completed download dialog should be shown. - */ - fun shouldShowCompletedDownloadDialog( - downloadState: DownloadState, - status: Status, - currentTab: SessionState?, - ): Boolean { - val isValidStatus = - status in listOf(Status.COMPLETED, Status.FAILED) - val isSameTab = downloadState.sessionId == (currentTab?.id ?: false) - - return isValidStatus && isSameTab - } -} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadUtils.kt b/fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadUtils.kt new file mode 100644 index 000000000000..ed0930f764b6 --- /dev/null +++ b/fenix/app/src/main/java/org/mozilla/fenix/browser/DownloadUtils.kt @@ -0,0 +1,53 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.fenix.browser + +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.state.content.DownloadState.Status +import mozilla.components.feature.downloads.AbstractFetchDownloadService +import org.mozilla.fenix.R +import org.mozilla.fenix.downloads.DynamicDownloadDialog + +internal fun BaseBrowserFragment.handleOnDownloadFinished( + downloadState: DownloadState, + downloadJobStatus: Status, + tryAgain: (String) -> Unit, +) { + // If the download is just paused, don't show any in-app notification + if (shouldShowCompletedDownloadDialog(downloadState, downloadJobStatus)) { + val safeContext = context ?: return + val onCannotOpenFile: (DownloadState) -> Unit = { + showCannotOpenFileError(binding.dynamicSnackbarContainer, safeContext, it) + } + if (downloadState.openInApp && downloadJobStatus == Status.COMPLETED) { + val fileWasOpened = AbstractFetchDownloadService.openFile( + applicationContext = safeContext.applicationContext, + download = downloadState, + ) + if (!fileWasOpened) { + onCannotOpenFile(downloadState) + } + } else { + saveDownloadDialogState( + downloadState.sessionId, + downloadState, + downloadJobStatus, + ) + + val dynamicDownloadDialog = DynamicDownloadDialog( + context = safeContext, + downloadState = downloadState, + didFail = downloadJobStatus == Status.FAILED, + tryAgain = tryAgain, + onCannotOpenFile = onCannotOpenFile, + binding = binding.viewDynamicDownloadDialog, + toolbarHeight = resources.getDimensionPixelSize(R.dimen.browser_toolbar_height), + ) { sharedViewModel.downloadDialogState.remove(downloadState.sessionId) } + + dynamicDownloadDialog.show() + browserToolbarView.expand() + } + } +} diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt index 4c806ae86476..0f6f107df176 100644 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt +++ b/fenix/app/src/main/java/org/mozilla/fenix/components/Components.kt @@ -16,7 +16,6 @@ import mozilla.components.feature.addons.amo.AMOAddonsProvider import mozilla.components.feature.addons.migration.DefaultSupportedAddonsChecker import mozilla.components.feature.addons.update.DefaultAddonUpdater import mozilla.components.feature.autofill.AutofillConfiguration -import mozilla.components.feature.downloads.manager.FetchDownloadManager import mozilla.components.lib.publicsuffixlist.PublicSuffixList import mozilla.components.support.base.android.NotificationsDelegate import mozilla.components.support.base.worker.Frequency @@ -30,7 +29,6 @@ import org.mozilla.fenix.autofill.AutofillUnlockActivity import org.mozilla.fenix.components.appstate.AppState import org.mozilla.fenix.components.metrics.MetricsMiddleware import org.mozilla.fenix.datastore.pocketStoriesSelectedCategoriesDataStore -import org.mozilla.fenix.downloads.DownloadService import org.mozilla.fenix.ext.asRecentTabs import org.mozilla.fenix.ext.components import org.mozilla.fenix.ext.filterState @@ -101,15 +99,6 @@ class Components(private val context: Context) { ) } - val downloadManager by lazyMonitored { - FetchDownloadManager( - context.applicationContext, - core.store, - DownloadService::class, - notificationsDelegate = notificationsDelegate, - ) - } - val intentProcessors by lazyMonitored { IntentProcessors( context, diff --git a/fenix/app/src/main/java/org/mozilla/fenix/components/DownloadStyling.kt b/fenix/app/src/main/java/org/mozilla/fenix/components/DownloadStyling.kt deleted file mode 100644 index 199f2312b8c2..000000000000 --- a/fenix/app/src/main/java/org/mozilla/fenix/components/DownloadStyling.kt +++ /dev/null @@ -1,36 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.components - -import android.content.Context -import android.view.Gravity -import mozilla.components.feature.downloads.DownloadsFeature -import org.mozilla.fenix.R -import org.mozilla.fenix.theme.ThemeManager - -/** - * Provides access to all Fenix download styling. - */ -object DownloadStyling { - - /** - * creates [DownloadsFeature.PromptsStyling]. - */ - fun createPrompt(context: Context): DownloadsFeature.PromptsStyling { - return DownloadsFeature.PromptsStyling( - gravity = Gravity.BOTTOM, - shouldWidthMatchParent = true, - positiveButtonBackgroundColor = ThemeManager.resolveAttribute( - R.attr.accent, - context, - ), - positiveButtonTextColor = ThemeManager.resolveAttribute( - R.attr.textOnColorPrimary, - context, - ), - positiveButtonRadius = (context.resources.getDimensionPixelSize(R.dimen.tab_corner_radius)).toFloat(), - ) - } -} diff --git a/fenix/app/src/main/res/layout/fragment_add_on_internal_settings.xml b/fenix/app/src/main/res/layout/fragment_add_on_internal_settings.xml index 0b3fa0573f3f..282f5bd3b419 100644 --- a/fenix/app/src/main/res/layout/fragment_add_on_internal_settings.xml +++ b/fenix/app/src/main/res/layout/fragment_add_on_internal_settings.xml @@ -13,18 +13,4 @@ android:id="@+id/addonSettingsEngineView" android:layout_width="match_parent" android:layout_height="match_parent" /> - - - - diff --git a/fenix/app/src/test/java/org/mozilla/fenix/addons/AddonPopupBaseFragmentTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/addons/AddonPopupBaseFragmentTest.kt deleted file mode 100644 index 28fc9e2612ed..000000000000 --- a/fenix/app/src/test/java/org/mozilla/fenix/addons/AddonPopupBaseFragmentTest.kt +++ /dev/null @@ -1,53 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.addons - -import io.mockk.every -import io.mockk.mockk -import io.mockk.spyk -import io.mockk.verify -import mozilla.components.browser.state.action.ContentAction -import mozilla.components.browser.state.state.SessionState -import mozilla.components.browser.state.store.BrowserStore -import mozilla.components.concept.fetch.Response -import org.junit.Before -import org.junit.Test -import org.junit.runner.RunWith -import org.mozilla.fenix.helpers.FenixRobolectricTestRunner - -@RunWith(FenixRobolectricTestRunner::class) -class AddonPopupBaseFragmentTest { - - private lateinit var fragment: AddonPopupBaseFragment - - @Before - fun setup() { - fragment = spyk() - } - - @Test - fun `WHEN onExternalResource is call THEN dispatch an UpdateDownloadAction`() { - val store = mockk(relaxed = true) - val session = mockk(relaxed = true) - val response = mockk(relaxed = true) - every { fragment.provideBrowserStore() } returns store - - fragment.session = session - - fragment.onExternalResource( - url = "url", - fileName = "fileName", - contentLength = 1, - contentType = "contentType", - userAgent = "userAgent", - isPrivate = true, - skipConfirmation = false, - openInApp = false, - response = response, - ) - - verify { store.dispatch(any()) } - } -} diff --git a/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt index 4a7e37e36c10..ac0a109f9e98 100644 --- a/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt +++ b/fenix/app/src/test/java/org/mozilla/fenix/browser/BaseBrowserFragmentTest.kt @@ -12,7 +12,11 @@ import io.mockk.mockk import io.mockk.slot import io.mockk.spyk import io.mockk.verify +import junit.framework.TestCase.assertFalse +import junit.framework.TestCase.assertTrue import mozilla.components.browser.state.state.SessionState +import mozilla.components.browser.state.state.content.DownloadState +import mozilla.components.browser.state.state.createTab import mozilla.components.concept.engine.EngineView import mozilla.components.concept.engine.permission.SitePermissions import mozilla.components.feature.contextmenu.ContextMenuCandidate @@ -104,7 +108,6 @@ class BaseBrowserFragmentTest { } @Test - @Suppress("ktlint:standard:max-line-length") fun `initializeEngineView should set toolbar height as EngineView parent's bottom margin when using bottom toolbar`() { every { settings.isDynamicToolbarEnabled } returns false every { settings.shouldUseBottomToolbar } returns true @@ -115,7 +118,7 @@ class BaseBrowserFragmentTest { } @Test - fun `initializeEngineView set toolbar height as EngineView parent's bottom margin if top toolbar`() { + fun `initializeEngineView should set toolbar height as EngineView parent's bottom margin if top toolbar is forced for a11y`() { every { settings.shouldUseBottomToolbar } returns false every { settings.shouldUseFixedTopToolbar } returns true @@ -125,8 +128,7 @@ class BaseBrowserFragmentTest { } @Test - @Suppress("MaxLineLength") - fun `initializeEngineView set toolbar height as EngineView parent's bottom margin if bottom toolbar`() { + fun `initializeEngineView should set toolbar height as EngineView parent's bottom margin if bottom toolbar is forced for a11y`() { every { settings.shouldUseBottomToolbar } returns true every { settings.shouldUseFixedTopToolbar } returns true @@ -134,6 +136,69 @@ class BaseBrowserFragmentTest { verify { (swipeRefreshLayout.layoutParams as CoordinatorLayout.LayoutParams).bottomMargin = 13 } } + + @Test + fun `WHEN status is equals to FAILED or COMPLETED and it is the same tab then shouldShowCompletedDownloadDialog will be true`() { + every { fragment.getCurrentTab() } returns createTab(id = "1", url = "") + + val download = DownloadState( + url = "", + sessionId = "1", + destinationDirectory = "/", + ) + + val status = DownloadState.Status.values() + .filter { it == DownloadState.Status.COMPLETED && it == DownloadState.Status.FAILED } + + status.forEach { + val result = + fragment.shouldShowCompletedDownloadDialog(download, it) + + assertTrue(result) + } + } + + @Test + fun `WHEN status is different from FAILED or COMPLETED then shouldShowCompletedDownloadDialog will be false`() { + every { fragment.getCurrentTab() } returns createTab(id = "1", url = "") + + val download = DownloadState( + url = "", + sessionId = "1", + destinationDirectory = "/", + ) + + val status = DownloadState.Status.values() + .filter { it != DownloadState.Status.COMPLETED && it != DownloadState.Status.FAILED } + + status.forEach { + val result = + fragment.shouldShowCompletedDownloadDialog(download, it) + + assertFalse(result) + } + } + + @Test + fun `WHEN the tab is different from the initial one then shouldShowCompletedDownloadDialog will be false`() { + every { fragment.getCurrentTab() } returns createTab(id = "1", url = "") + + val download = DownloadState( + url = "", + sessionId = "2", + destinationDirectory = "/", + ) + + val status = DownloadState.Status.values() + .filter { it != DownloadState.Status.COMPLETED && it != DownloadState.Status.FAILED } + + status.forEach { + val result = + fragment.shouldShowCompletedDownloadDialog(download, it) + + assertFalse(result) + } + } } private class TestBaseBrowserFragment : BaseBrowserFragment() { diff --git a/fenix/app/src/test/java/org/mozilla/fenix/browser/DownloadDialogUtilsTest.kt b/fenix/app/src/test/java/org/mozilla/fenix/browser/DownloadDialogUtilsTest.kt deleted file mode 100644 index fbd3d013e26c..000000000000 --- a/fenix/app/src/test/java/org/mozilla/fenix/browser/DownloadDialogUtilsTest.kt +++ /dev/null @@ -1,173 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this - * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ - -package org.mozilla.fenix.browser - -import androidx.test.ext.junit.runners.AndroidJUnit4 -import mozilla.components.browser.state.state.content.DownloadState -import mozilla.components.browser.state.state.content.DownloadState.Status.COMPLETED -import mozilla.components.browser.state.state.content.DownloadState.Status.FAILED -import mozilla.components.browser.state.state.createTab -import mozilla.components.support.test.robolectric.testContext -import org.junit.Assert.assertFalse -import org.junit.Assert.assertTrue -import org.junit.Test -import org.junit.runner.RunWith - -@RunWith(AndroidJUnit4::class) -class DownloadDialogUtilsTest { - - @Test - @Suppress("MaxLineLength") - fun `GIVEN final status WHEN handleOnDownloadFinished try to open the file fails THEN show error message`() { - val downloadUtils = DownloadDialogUtils - val currentTab = createTab(id = "1", url = "") - val download = DownloadState( - url = "", - sessionId = currentTab.id, - destinationDirectory = "/", - openInApp = true, - ) - var dialogWasShown = false - var onCannotOpenFileWasCalled = false - - downloadUtils.openFile = { _, _ -> false } - - downloadUtils.handleOnDownloadFinished( - context = testContext, - downloadState = download, - downloadJobStatus = COMPLETED, - currentTab = currentTab, - onFinishedDialogShown = { dialogWasShown = true }, - onCannotOpenFile = { - onCannotOpenFileWasCalled = true - }, - ) - - assertTrue(onCannotOpenFileWasCalled) - assertFalse(dialogWasShown) - } - - @Test - fun `GIVEN final status and openInApp WHEN calling handleOnDownloadFinished THEN try to open the file`() { - val downloadUtils = DownloadDialogUtils - val download = DownloadState( - url = "", - sessionId = "1", - destinationDirectory = "/", - openInApp = true, - ) - val currentTab = createTab(id = "1", url = "") - var dialogWasShown = false - var onCannotOpenFileWasCalled = false - - downloadUtils.openFile = { _, _ -> true } - - downloadUtils.handleOnDownloadFinished( - context = testContext, - downloadState = download, - downloadJobStatus = COMPLETED, - currentTab = currentTab, - onFinishedDialogShown = { dialogWasShown = true }, - onCannotOpenFile = { - onCannotOpenFileWasCalled = true - }, - ) - - assertFalse(onCannotOpenFileWasCalled) - assertFalse(dialogWasShown) - } - - @Test - fun `GIVEN final status WHEN calling handleOnDownloadFinished THEN show a finished download dialog`() { - val downloadUtils = DownloadDialogUtils - val download = DownloadState( - url = "", - sessionId = "1", - destinationDirectory = "/", - ) - val currentTab = createTab(id = "1", url = "") - var dialogWasShown = false - - downloadUtils.handleOnDownloadFinished( - context = testContext, - downloadState = download, - downloadJobStatus = COMPLETED, - currentTab = currentTab, - onFinishedDialogShown = { dialogWasShown = true }, - onCannotOpenFile = {}, - ) - - assertTrue(dialogWasShown) - } - - @Test - fun `WHEN status is final and is same tab THEN shouldShowCompletedDownloadDialog will be true`() { - val currentTab = createTab(id = "1", url = "") - - val download = DownloadState( - url = "", - sessionId = "1", - destinationDirectory = "/", - ) - - val status = DownloadState.Status.values().filter { it == COMPLETED || it == FAILED } - - status.forEach { - val result = DownloadDialogUtils.shouldShowCompletedDownloadDialog( - downloadState = download, - status = it, - currentTab = currentTab, - ) - - assertTrue(result) - } - } - - @Test - fun `WHEN status is different from FAILED or COMPLETED then shouldShowCompletedDownloadDialog will be false`() { - val currentTab = createTab(id = "1", url = "") - - val download = DownloadState( - url = "", - sessionId = "1", - destinationDirectory = "/", - ) - - val completedStatus = listOf(COMPLETED, FAILED) - val status = DownloadState.Status.values().filter { it !in completedStatus } - - status.forEach { - val result = DownloadDialogUtils.shouldShowCompletedDownloadDialog( - downloadState = download, - status = it, - currentTab = currentTab, - ) - - assertFalse(result) - } - } - - @Test - fun `WHEN the tab is different from the initial one then shouldShowCompletedDownloadDialog will be false`() { - val currentTab = createTab(id = "1", url = "") - - val download = DownloadState( - url = "", - sessionId = "2", - destinationDirectory = "/", - ) - val completedStatus = listOf(COMPLETED, FAILED) - val status = DownloadState.Status.values().filter { it !in completedStatus } - status.forEach { - val result = - DownloadDialogUtils.shouldShowCompletedDownloadDialog( - downloadState = download, - status = it, - currentTab = currentTab, - ) - assertFalse(result) - } - } -} From bbc1bd60c15bb31ba4ae700c386075ed90c98cab Mon Sep 17 00:00:00 2001 From: ohall-m Date: Wed, 13 Mar 2024 10:21:59 -0400 Subject: [PATCH 3/6] Bug 1885137 - Added Logging for if the Translation Engine is Supported Added additional logging to know if the translations engine is supported on the device. --- .../browser/state/engine/middleware/TranslationsMiddleware.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt index 2b9180c8e04c..5d9d77051fc1 100644 --- a/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt +++ b/android-components/components/browser/state/src/main/java/mozilla/components/browser/state/engine/middleware/TranslationsMiddleware.kt @@ -198,7 +198,7 @@ class TranslationsMiddleware( isEngineSupported = isEngineSupported, ), ) - logger.info("Success requesting engine support.") + logger.info("Success requesting engine support. isEngineSupported: $isEngineSupported") continuation.resume(isEngineSupported) }, From 245362f20abb7e1c2e079d50f22d73ee2f205bd2 Mon Sep 17 00:00:00 2001 From: "oana.horvath" Date: Wed, 13 Mar 2024 17:10:18 +0200 Subject: [PATCH 4/6] Bug 1885160 - Add logs to AppAndSystemHelper --- .../fenix/helpers/AppAndSystemHelper.kt | 158 ++++++++++++++---- .../org/mozilla/fenix/ui/robots/PwaRobot.kt | 1 + 2 files changed, 129 insertions(+), 30 deletions(-) diff --git a/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/AppAndSystemHelper.kt b/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/AppAndSystemHelper.kt index 35c2f5c7d034..ac40972db6cd 100644 --- a/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/AppAndSystemHelper.kt +++ b/fenix/app/src/androidTest/java/org/mozilla/fenix/helpers/AppAndSystemHelper.kt @@ -46,6 +46,7 @@ import org.mozilla.fenix.customtabs.ExternalAppBrowserActivity import org.mozilla.fenix.helpers.Constants.PackageName.PIXEL_LAUNCHER import org.mozilla.fenix.helpers.Constants.PackageName.YOUTUBE_APP import org.mozilla.fenix.helpers.Constants.TAG +import org.mozilla.fenix.helpers.TestAssetHelper.waitingTime import org.mozilla.fenix.helpers.TestAssetHelper.waitingTimeShort import org.mozilla.fenix.helpers.TestHelper.appContext import org.mozilla.fenix.helpers.TestHelper.mDevice @@ -61,10 +62,17 @@ object AppAndSystemHelper { private val bookmarksStorage = PlacesBookmarksStorage(appContext.applicationContext) suspend fun bookmarks() = bookmarksStorage.getTree(BookmarkRoot.Mobile.id)?.children fun getPermissionAllowID(): String { + Log.i(TAG, "getPermissionAllowID: Trying to get the permission button resource ID based on API.") return when (Build.VERSION.SDK_INT > Build.VERSION_CODES.P) { - true -> "com.android.permissioncontroller" - false -> "com.android.packageinstaller" + true -> { + Log.i(TAG, "getPermissionAllowID: Getting the permission button resource ID for API ${Build.VERSION.SDK_INT}.") + "com.android.permissioncontroller" + } + false -> { + Log.i(TAG, "getPermissionAllowID: Getting the permission button resource ID for API ${Build.VERSION.SDK_INT}.") + "com.android.packageinstaller" + } } } @@ -76,6 +84,7 @@ object AppAndSystemHelper { */ fun deleteDownloadedFileOnStorage(fileName: String) { if (Build.VERSION.SDK_INT > Build.VERSION_CODES.Q) { + Log.i(TAG, "deleteDownloadedFileOnStorage: Trying to delete file from API ${Build.VERSION.SDK_INT}.") val storageManager: StorageManager? = appContext.getSystemService(Context.STORAGE_SERVICE) as StorageManager? val storageVolumes = storageManager!!.storageVolumes @@ -83,24 +92,28 @@ object AppAndSystemHelper { val file = File(storageVolume.directory!!.path + "/Download/" + fileName) try { if (file.exists()) { + Log.i(TAG, "deleteDownloadedFileOnStorage: The file exists. Trying to delete $fileName, try 1.") file.delete() - Log.d("TestLog", "File delete try 1") - Assert.assertFalse("The file was not deleted", file.exists()) + Assert.assertFalse("$TAG deleteDownloadedFileOnStorage: The $fileName file was not deleted", file.exists()) + Log.i(TAG, "deleteDownloadedFileOnStorage: Verified the $fileName file was deleted.") } } catch (e: AssertionError) { + Log.i(TAG, "deleteDownloadedFileOnStorage: AssertionError caught. Retrying to delete the file.") file.delete() - Log.d("TestLog", "File delete retried") - Assert.assertFalse("The file was not deleted", file.exists()) + Log.i(TAG, "deleteDownloadedFileOnStorage: Retrying to delete $fileName.") + Assert.assertFalse("$TAG deleteDownloadedFileOnStorage: The file was not deleted", file.exists()) + Log.i(TAG, "deleteDownloadedFileOnStorage: Verified the $fileName file was deleted, try 2.") } } else { runBlocking { + Log.i(TAG, "deleteDownloadedFileOnStorage: Trying to delete file from API ${Build.VERSION.SDK_INT}.") val downloadedFile = File( Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS), fileName, ) if (downloadedFile.exists()) { - Log.i(TAG, "deleteDownloadedFileOnStorage: Verifying if $downloadedFile exists.") + Log.i(TAG, "deleteDownloadedFileOnStorage: The file exists. Trying to delete the file.") downloadedFile.delete() Log.i(TAG, "deleteDownloadedFileOnStorage: $downloadedFile deleted.") } @@ -114,8 +127,8 @@ object AppAndSystemHelper { * Environment.getExternalStorageDirectory() is deprecated starting with API 29. */ fun clearDownloadsFolder() { + Log.i(TAG, "clearDownloadsFolder: Detected API ${Build.VERSION.SDK_INT}") if (Build.VERSION.SDK_INT > Build.VERSION_CODES.Q) { - Log.i(TAG, "clearDownloadsFolder: API > 29") val storageManager: StorageManager? = appContext.getSystemService(Context.STORAGE_SERVICE) as StorageManager? val storageVolumes = storageManager!!.storageVolumes @@ -135,6 +148,10 @@ object AppAndSystemHelper { ) // Delete all files in the folder for (file in files!!) { + Log.i( + TAG, + "clearDownloadsFolder: Trying to delete $file from \"DOWNLOADS\" folder.", + ) file.delete() Log.i( TAG, @@ -155,11 +172,10 @@ object AppAndSystemHelper { } } else { runBlocking { - Log.i(TAG, "clearDownloadsFolder: API <= 29") Log.i(TAG, "clearDownloadsFolder: Verifying if any download files exist.") Environment.getExternalStoragePublicDirectory(Environment.DIRECTORY_DOWNLOADS) .listFiles()?.forEach { - Log.i(TAG, "clearDownloadsFolder: Downloads storage contains: $it.") + Log.i(TAG, "clearDownloadsFolder: Trying to delete from storage: $it.") it.delete() Log.i(TAG, "clearDownloadsFolder: Download file $it deleted.") } @@ -220,27 +236,53 @@ object AppAndSystemHelper { when (enabled) { true -> { + Log.i( + TAG, + "setNetworkEnabled: Trying to enable the network connection.", + ) mDevice.executeShellCommand("svc data enable") + Log.i( + TAG, + "setNetworkEnabled: Data network connection enable command sent.", + ) mDevice.executeShellCommand("svc wifi enable") + Log.i( + TAG, + "setNetworkEnabled: Wifi network connection enable command sent.", + ) // Wait for network connection to be completely enabled + Log.i(TAG, "setNetworkEnabled: Waiting for connection to be enabled.") IdlingRegistry.getInstance().register(networkConnectedIdlingResource) Espresso.onIdle { IdlingRegistry.getInstance().unregister(networkConnectedIdlingResource) } - Log.i(TAG, "setNetworkEnabled: Network connection was enabled") + Log.i(TAG, "setNetworkEnabled: Network connection was enabled.") } false -> { + Log.i( + TAG, + "setNetworkEnabled: Trying to disable the network connection.", + ) mDevice.executeShellCommand("svc data disable") + Log.i( + TAG, + "setNetworkEnabled: Data network connection disable command sent.", + ) mDevice.executeShellCommand("svc wifi disable") + Log.i( + TAG, + "setNetworkEnabled: Wifi network connection disable command sent.", + ) // Wait for network connection to be completely disabled + Log.i(TAG, "setNetworkEnabled: Waiting for connection to be disabled.") IdlingRegistry.getInstance().register(networkDisconnectedIdlingResource) Espresso.onIdle { IdlingRegistry.getInstance().unregister(networkDisconnectedIdlingResource) } - Log.i(TAG, "setNetworkEnabled: Network connection was disabled") + Log.i(TAG, "setNetworkEnabled: Network connection was disabled.") } } } @@ -250,6 +292,8 @@ object AppAndSystemHelper { return try { val packageManager = InstrumentationRegistry.getInstrumentation().context.packageManager packageManager.getApplicationInfo(packageName, 0).enabled + Log.i(TAG, "isPackageInstalled: $packageName is installed.") + true } catch (e: PackageManager.NameNotFoundException) { Log.i(TAG, "isPackageInstalled: $packageName is not installed - ${e.message}") false @@ -258,18 +302,18 @@ object AppAndSystemHelper { fun assertExternalAppOpens(appPackageName: String) { if (isPackageInstalled(appPackageName)) { - Log.i(TAG, "assertExternalAppOpens: $appPackageName is installed on device") try { - Log.i(TAG, "assertExternalAppOpens: Try block") + Log.i(TAG, "assertExternalAppOpens: Trying to check the intent sent.") intended(toPackage(appPackageName)) - Log.i(TAG, "assertExternalAppOpens: Matched intent to $appPackageName") + Log.i(TAG, "assertExternalAppOpens: Matched open intent to $appPackageName.") } catch (e: AssertionFailedError) { - Log.i(TAG, "assertExternalAppOpens: Catch block - ${e.message}") + Log.i(TAG, "assertExternalAppOpens: Intent match failure. ${e.message}") } } else { + Log.i(TAG, "assertExternalAppOpens: Trying to verify the \"Could not open file\" message.") mDevice.waitNotNull( Until.findObject(By.text("Could not open file")), - TestAssetHelper.waitingTime, + waitingTime, ) Log.i(TAG, "assertExternalAppOpens: Verified \"Could not open file\" message") } @@ -277,17 +321,28 @@ object AppAndSystemHelper { fun assertNativeAppOpens(appPackageName: String, url: String = "") { if (isPackageInstalled(appPackageName)) { - mDevice.waitForIdle(TestAssetHelper.waitingTimeShort) + Log.i(TAG, "assertNativeAppOpens: Waiting for the device to be idle $waitingTimeShort ms.") + mDevice.waitForIdle(waitingTimeShort) + Log.i(TAG, "assertNativeAppOpens: Waited for the device to be idle $waitingTimeShort ms.") + Log.i(TAG, "assertNativeAppOpens: Trying to match the app package name is matched.") Assert.assertTrue( - TestHelper.mDevice.findObject(UiSelector().packageName(appPackageName)) - .waitForExists(TestAssetHelper.waitingTime), + "$TAG $appPackageName not found", + mDevice.findObject(UiSelector().packageName(appPackageName)) + .waitForExists(waitingTime), ) + Log.i(TAG, "assertNativeAppOpens: App package name matched.") } else { + Log.i(TAG, "assertNativeAppOpens: Trying to verify the page redirect URL.") BrowserRobot().verifyUrl(url) + Log.i(TAG, "assertNativeAppOpens: Verified the page redirect URL.") } } - fun assertYoutubeAppOpens() = intended(toPackage(YOUTUBE_APP)) + fun assertYoutubeAppOpens() { + Log.i(TAG, "assertYoutubeAppOpens: Trying to check the intent to YouTube.") + intended(toPackage(YOUTUBE_APP)) + Log.i(TAG, "assertYoutubeAppOpens: Verified the intent matches YouTube.") + } /** * Checks whether the latest activity of the application is used for custom tabs or PWAs. @@ -295,11 +350,16 @@ object AppAndSystemHelper { * @return Boolean value that helps us know if the current activity supports custom tabs or PWAs. */ fun isExternalAppBrowserActivityInCurrentTask(): Boolean { - Log.i(TAG, "Trying to verify that the latest activity of the application is used for custom tabs or PWAs") val activityManager = appContext.getSystemService(Context.ACTIVITY_SERVICE) as ActivityManager - mDevice.waitForIdle(TestAssetHelper.waitingTimeShort) + Log.i(TAG, "isExternalAppBrowserActivityInCurrentTask: Waiting for the device to be idle for $waitingTimeShort ms") + mDevice.waitForIdle(waitingTimeShort) + Log.i(TAG, "isExternalAppBrowserActivityInCurrentTask: Waited for the device to be idle for $waitingTimeShort ms") + Log.i( + TAG, + "isExternalAppBrowserActivityInCurrentTask: Trying to verify that the latest activity of the application is used for custom tabs or PWAs", + ) return activityManager.appTasks[0].taskInfo.topActivity!!.className == ExternalAppBrowserActivity::class.java.name } @@ -314,14 +374,18 @@ object AppAndSystemHelper { testBlock: () -> Unit, ) { idlingResources.forEach { + Log.i(TAG, "registerAndCleanupIdlingResources: Trying to register idling resource $it.") IdlingRegistry.getInstance().register(it) + Log.i(TAG, "registerAndCleanupIdlingResources: Registered idling resource $it.") } try { testBlock() } finally { idlingResources.forEach { + Log.i(TAG, "registerAndCleanupIdlingResources: Trying to unregister idling resource $it.") IdlingRegistry.getInstance().unregister(it) + Log.i(TAG, "registerAndCleanupIdlingResources: Unregistered idling resource $it.") } } } @@ -339,18 +403,26 @@ object AppAndSystemHelper { ) if (Build.VERSION.SDK_INT >= 23) { - if (whileUsingTheAppPermissionButton.waitForExists(TestAssetHelper.waitingTimeShort)) { + if (whileUsingTheAppPermissionButton.waitForExists(waitingTimeShort)) { + Log.i(TAG, "grantSystemPermission: Trying to click the \"While using the app\" button.") whileUsingTheAppPermissionButton.click() - } else if (allowPermissionButton.waitForExists(TestAssetHelper.waitingTimeShort)) { + Log.i(TAG, "grantSystemPermission: Clicked the \"While using the app\" button.") + } else if (allowPermissionButton.waitForExists(waitingTimeShort)) { + Log.i(TAG, "grantSystemPermission: Trying to click the \"Allow\" button.") allowPermissionButton.click() + Log.i(TAG, "grantSystemPermission: Clicked the \"Allow\" button.") } } } // Permission deny dialogs differ on various Android APIs fun denyPermission() { - mDevice.findObject(UiSelector().textContains("Deny")).waitForExists(TestAssetHelper.waitingTime) + Log.i(TAG, "denyPermission: Waiting $waitingTime ms for the \"Deny\" button to exist.") + mDevice.findObject(UiSelector().textContains("Deny")).waitForExists(waitingTime) + Log.i(TAG, "denyPermission: Waited for $waitingTime ms for the \"Deny\" button to exist.") + Log.i(TAG, "denyPermission: Trying to click the \"Deny\" button.") mDevice.findObject(UiSelector().textContains("Deny")).click() + Log.i(TAG, "denyPermission: Clicked the \"Deny\" button.") } fun isTestLab(): Boolean { @@ -366,12 +438,17 @@ object AppAndSystemHelper { val defaultLocale = Locale.getDefault() try { + Log.i(TAG, "runWithSystemLocaleChanged: Trying to set the locale.") setSystemLocale(locale) + Log.i(TAG, "runWithSystemLocaleChanged: Running the test block.") testBlock() ThreadUtils.runOnUiThread { testRule.activity.recreate() } + Log.i(TAG, "runWithSystemLocaleChanged: Test block finished.") } catch (e: Exception) { + Log.i(TAG, "runWithSystemLocaleChanged: The test block has thrown an exception.${e.message}") e.printStackTrace() } finally { + Log.i(TAG, "runWithSystemLocaleChanged: Trying to reset the locale to default.") setSystemLocale(defaultLocale) } } @@ -416,10 +493,14 @@ object AppAndSystemHelper { } fun putAppToBackground() { + Log.i(TAG, "putAppToBackground: Trying to press the device Recent apps button.") mDevice.pressRecentApps() + Log.i(TAG, "putAppToBackground: Pressed the device Recent apps button.") + Log.i(TAG, "putAppToBackground: Waiting for the app to be gone for $waitingTime ms.") mDevice.findObject(UiSelector().resourceId("${TestHelper.packageName}:id/container")).waitUntilGone( - TestAssetHelper.waitingTime, + waitingTime, ) + Log.i(TAG, "putAppToBackground: Waited for the app to be gone for $waitingTime ms.") } /** @@ -428,12 +509,19 @@ object AppAndSystemHelper { * The recent apps tray on API 30 will always display only 2 apps, even if previously were opened more. * The index of the most recent opened app will always have index 2, meaning that the previously opened app will have index 1. */ - fun bringAppToForeground() = - mDevice.findObject(UiSelector().index(2).packageName(PIXEL_LAUNCHER)).clickAndWaitForNewWindow(waitingTimeShort) + fun bringAppToForeground() { + Log.i(TAG, "bringAppToForeground: Trying to select the app from the recent apps tray and wait for $waitingTime ms for a new window") + mDevice.findObject(UiSelector().index(2).packageName(PIXEL_LAUNCHER)) + .clickAndWaitForNewWindow(waitingTimeShort) + Log.i(TAG, "bringAppToForeground: Selected the app from the recent apps tray.") + } fun verifyKeyboardVisibility(isExpectedToBeVisible: Boolean = true) { + Log.i(TAG, "verifyKeyboardVisibility: Waiting for the device to be idle.") mDevice.waitForIdle() + Log.i(TAG, "verifyKeyboardVisibility: Waited for the device to be idle.") + Log.i(TAG, "verifyKeyboardVisibility: Trying to verify the keyboard is visible.") assertEquals( "Keyboard not shown", isExpectedToBeVisible, @@ -441,6 +529,7 @@ object AppAndSystemHelper { .executeShellCommand("dumpsys input_method | grep mInputShown") .contains("mInputShown=true"), ) + Log.i(TAG, "verifyKeyboardVisibility: Verified the keyboard is visible.") } fun openAppFromExternalLink(url: String) { @@ -452,10 +541,14 @@ object AppAndSystemHelper { flags = Intent.FLAG_ACTIVITY_NEW_TASK } try { + Log.i(TAG, "openAppFromExternalLink: Trying to start the activity from an external intent.") context.startActivity(intent) + Log.i(TAG, "openAppFromExternalLink: Activity started from an external intent.") } catch (ex: ActivityNotFoundException) { + Log.i(TAG, "openAppFromExternalLink: Exception caught. Trying to start the activity from a null intent.") intent.setPackage(null) context.startActivity(intent) + Log.i(TAG, "openAppFromExternalLink: Started the activity from a null intent.") } } @@ -464,6 +557,7 @@ object AppAndSystemHelper { * For example: this method will avoid accidentally running a test on GV versions where the feature is disabled. */ fun runWithCondition(condition: Boolean, testBlock: () -> Unit) { + Log.i(TAG, "runWithCondition: Trying to run the test based on condition. The condition is: $condition.") if (condition) { testBlock() } @@ -480,11 +574,15 @@ object AppAndSystemHelper { addCategory(Intent.CATEGORY_LAUNCHER) } + Log.i(TAG, "runWithLauncherIntent: Trying to launch the activity from an intent: $launcherIntent.") activityTestRule.activityRule.withIntent(launcherIntent).launchActivity(launcherIntent) - + Log.i(TAG, "runWithLauncherIntent: Launched the activity from an intent: $launcherIntent.") try { + Log.i(TAG, "runWithLauncherIntent: Trying run the test block.") testBlock() + Log.i(TAG, "runWithLauncherIntent: Finished running the test block.") } catch (e: Exception) { + Log.i(TAG, "runWithLauncherIntent: Exception caught while running the test block: ${e.message}") e.printStackTrace() } } diff --git a/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/robots/PwaRobot.kt b/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/robots/PwaRobot.kt index cd31c2fc8628..6456eadccf31 100644 --- a/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/robots/PwaRobot.kt +++ b/fenix/app/src/androidTest/java/org/mozilla/fenix/ui/robots/PwaRobot.kt @@ -19,6 +19,7 @@ class PwaRobot { fun verifyCustomTabToolbarIsNotDisplayed() = assertUIObjectExists(itemWithResId("$packageName:id/toolbar"), exists = false) fun verifyPwaActivityInCurrentTask() { assertTrue("$TAG: The latest activity of the application is not used for custom tabs or PWAs", isExternalAppBrowserActivityInCurrentTask()) + Log.i(TAG, "verifyPwaActivityInCurrentTask: Verified that the latest activity of the application is used for custom tabs or PWAs") } class Transition From 0392c921cd0c016a0a90938d66ff963fd6c9b9e6 Mon Sep 17 00:00:00 2001 From: JohanLorenzo Date: Wed, 13 Mar 2024 15:32:10 +0000 Subject: [PATCH 5/6] Update Fenix initial_experiments.json based on the current first-run experiments in experimenter --- .../src/main/res/raw/initial_experiments.json | 105 ++++++++++++++++++ 1 file changed, 105 insertions(+) diff --git a/fenix/app/src/main/res/raw/initial_experiments.json b/fenix/app/src/main/res/raw/initial_experiments.json index c771ed397786..1be096876015 100644 --- a/fenix/app/src/main/res/raw/initial_experiments.json +++ b/fenix/app/src/main/res/raw/initial_experiments.json @@ -377,6 +377,111 @@ "localizations": null, "locales": null, "publishedDate": "2024-03-12T15:23:02.314682Z" + }, + { + "schemaVersion": "1.12.0", + "slug": "splash-screen-max-duration-test-lower-times", + "id": "splash-screen-max-duration-test-lower-times", + "arguments": {}, + "application": "org.mozilla.firefox", + "appName": "fenix", + "appId": "org.mozilla.firefox", + "channel": "release", + "userFacingName": "Splash screen max duration test - lower times", + "userFacingDescription": "Testing a splashscreen on app launch.", + "isEnrollmentPaused": false, + "isRollout": false, + "bucketConfig": { + "randomizationUnit": "nimbus_id", + "namespace": "fenix-splash-screen-release-3", + "start": 0, + "count": 10000, + "total": 10000 + }, + "featureIds": [ + "splash-screen" + ], + "probeSets": [], + "outcomes": [ + { + "slug": "onboarding", + "priority": "primary" + }, + { + "slug": "default-browser", + "priority": "primary" + } + ], + "branches": [ + { + "slug": "control", + "ratio": 1, + "feature": { + "featureId": "this-is-included-for-mobile-pre-96-support", + "enabled": false, + "value": {} + }, + "features": [ + { + "featureId": "splash-screen", + "enabled": true, + "value": { + "enabled": true, + "maximum_duration_ms": 0 + } + } + ] + }, + { + "slug": "treatment-a", + "ratio": 1, + "feature": { + "featureId": "this-is-included-for-mobile-pre-96-support", + "enabled": false, + "value": {} + }, + "features": [ + { + "featureId": "splash-screen", + "enabled": true, + "value": { + "enabled": true, + "maximum_duration_ms": 1750 + } + } + ] + }, + { + "slug": "treatment-b", + "ratio": 1, + "feature": { + "featureId": "this-is-included-for-mobile-pre-96-support", + "enabled": false, + "value": {} + }, + "features": [ + { + "featureId": "splash-screen", + "enabled": true, + "value": { + "enabled": true, + "maximum_duration_ms": 2500 + } + } + ] + } + ], + "targeting": "((is_already_enrolled) || ((isFirstRun == 'true') && (app_version|versionCompare('124.!') >= 0) && (region in ['AD', 'AE', 'AF', 'AG', 'AI', 'AL', 'AM', 'AO', 'AQ', 'AR', 'AS', 'AT', 'AU', 'AW', 'AX', 'AZ', 'BA', 'BB', 'BD', 'BE', 'BF', 'BG', 'BH', 'BI', 'BJ', 'BL', 'BM', 'BN', 'BO', 'BQ', 'BR', 'BS', 'BT', 'BV', 'BW', 'BY', 'BZ', 'CA', 'CC', 'CD', 'CF', 'CG', 'CH', 'CI', 'CK', 'CL', 'CM', 'CN', 'CO', 'CR', 'CU', 'CV', 'CW', 'CX', 'CY', 'CZ', 'DJ', 'DK', 'DM', 'DO', 'DZ', 'EC', 'EE', 'EG', 'EH', 'ER', 'ES', 'ET', 'FI', 'FJ', 'FK', 'FM', 'FO', 'FR', 'GA', 'GB', 'GD', 'GE', 'GF', 'GG', 'GH', 'GI', 'GL', 'GM', 'GN', 'GP', 'GQ', 'GR', 'GS', 'GT', 'GU', 'GW', 'GY', 'HK', 'HM', 'HN', 'HR', 'HT', 'HU', 'ID', 'IE', 'IL', 'IM', 'IN', 'IO', 'IQ', 'IR', 'IS', 'IT', 'JE', 'JM', 'JO', 'JP', 'KE', 'KG', 'KH', 'KI', 'KM', 'KN', 'KP', 'KR', 'KW', 'KY', 'KZ', 'LA', 'LB', 'LC', 'LI', 'LK', 'LR', 'LS', 'LT', 'LU', 'LV', 'LY', 'MA', 'MC', 'MD', 'ME', 'MF', 'MG', 'MH', 'MK', 'ML', 'MM', 'MN', 'MO', 'MP', 'MQ', 'MR', 'MS', 'MT', 'MU', 'MV', 'MW', 'MX', 'MY', 'MZ', 'NA', 'NC', 'NE', 'NF', 'NG', 'NI', 'NL', 'NO', 'NP', 'NR', 'NU', 'NZ', 'OM', 'PA', 'PE', 'PF', 'PG', 'PH', 'PK', 'PL', 'PM', 'PN', 'PR', 'PS', 'PT', 'PW', 'PY', 'QA', 'RE', 'RO', 'RS', 'RU', 'RW', 'SA', 'SB', 'SC', 'SD', 'SE', 'SG', 'SH', 'SI', 'SJ', 'SK', 'SL', 'SM', 'SN', 'SO', 'SR', 'SS', 'ST', 'SV', 'SX', 'SY', 'SZ', 'TC', 'TD', 'TF', 'TG', 'TH', 'TJ', 'TK', 'TL', 'TM', 'TN', 'TO', 'TR', 'TT', 'TV', 'TW', 'TZ', 'UA', 'UG', 'UM', 'US', 'UY', 'UZ', 'VA', 'VC', 'VE', 'VG', 'VI', 'VN', 'VU', 'WF', 'WS', 'YE', 'YT', 'ZA', 'ZM', 'ZW'])))", + "startDate": "2024-03-13", + "enrollmentEndDate": null, + "endDate": null, + "proposedDuration": 56, + "proposedEnrollment": 28, + "referenceBranch": "control", + "featureValidationOptOut": false, + "localizations": null, + "locales": null, + "publishedDate": "2024-03-13T15:04:41.938148Z" } ] } From 024c3074de7533fbb83f3b7fc557ef450f0926ab Mon Sep 17 00:00:00 2001 From: Matthew Tighe Date: Fri, 9 Feb 2024 14:52:54 -0800 Subject: [PATCH 6/6] Bug 1879663 - Add RFC proposing updates to the RFC process --- docs/rfcs/0000-template.md | 46 +++++++++++++++++++++++++ docs/rfcs/0013-rfc-process-updates.md | 48 +++++++++++++++++++++++++++ docs/rfcs/README.md | 47 ++++++++++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 docs/rfcs/0000-template.md create mode 100644 docs/rfcs/0013-rfc-process-updates.md create mode 100644 docs/rfcs/README.md diff --git a/docs/rfcs/0000-template.md b/docs/rfcs/0000-template.md new file mode 100644 index 000000000000..4414c5b4ad84 --- /dev/null +++ b/docs/rfcs/0000-template.md @@ -0,0 +1,46 @@ +--- +layout: page +title: your title +permalink: /docs/rfcs/file-name +--- + +* RFC PR: [PR #](https://github.com/mozilla-mobile/android-components/pull/#) +* Start date: YYYY-MM-DD (Day of proposal posting.) +* End date: YYYY-MM-DD (Last day for general feedback. However, the proposal can be merged immediately after all stakeholders approve.) +* Stakeholders: github-username, github-username + +## Summary + +This section should include a brief description of the proposal. + +## Motivation + +This section should include reasoning about why the proposal is useful. Examples, specific scenarios, +open bugs, records of performance metrics, and other empirical data are beneficial to include here if available. + +## Guide-level explanation + +This section should include a high-level walkthrough of the steps required to implement the proposal. + +## Reference-level explanation (optional) + +This section should include a detailed walkthrough of technical steps or code changes that +will be required to implement the proposal. Code samples and prototypes are beneficial to include here. + +## Drawbacks (optional) + +This section should include any drawbacks to the proposal. + +## Rationale and alternatives + +This section should include any alternative proposals considered, as well as the rationale for why +they were not selected for the proposal. + +## Resources and Docs (optional) + +- Any (internal or external) similar proposals or other documentation that shares concepts with the proposal. +- Links to artifacts generated as part of the proposal, such as additional documentation or follow-up bugs. + +## Unresolved questions + +Questions from the proposal author or from reviewers that are not yet resolved. \ No newline at end of file diff --git a/docs/rfcs/0013-rfc-process-updates.md b/docs/rfcs/0013-rfc-process-updates.md new file mode 100644 index 000000000000..0d9fedc5da60 --- /dev/null +++ b/docs/rfcs/0013-rfc-process-updates.md @@ -0,0 +1,48 @@ +--- +layout: page +title: RFC process updates +permalink: /docs/rfcs/0013-rfc-process-updates +--- + +* RFC PR: https://github.com/mozilla-mobile/firefox-android/pull/5681 +* Start date: 2024-02-20 +* End date: 2024-03-22 +* Stakeholders: @jonalmeida, @boek + +## Summary + +Clarify requirements for RFCs by introducing a README and a template. + +## Motivation + +- The scale of the Firefox Android team has grown substantially since the RFC process was first introduced. +- More external teams are using and contributing to Android Components. +- Implicit deadlines have encouraged a lack of engagement with RFCs, slowing follow-up work. +- There has been a low level of engagement with RFCs because of a lack of clarity around the process and when they are appropriate. + +## Guide-level explanation + +This proposal suggests improving the RFC process by introducing the following: + +- An RFC template, to provide a clear starting point for proposals. +- A guide for when RFCs are appropriate and when they are not needed in the form of a README. +- A requirement for explicit stakeholders for RFCs. +- An initial deadline recorded in each proposal. +- A renaming of the the "Prior Art" section to "Resources and Docs" and wording to indicate that proposals can also include additional documentation. + +## Rationale and alternatives + +The RFC process has been successful in some cases, but has not been consistently followed. This proposal aims to make the process more accessible and clear, by introducing high-level concepts in the README and a clear template. These documents can be more easily treated as living documents, and updated with any future changes to the process. + +### Amend RFC 0001 with additional template information +- This was not included in the proposal in order to keep past RFCs consistent and free of modern context, and so that past RFCs do not become treated as living documents. + +## Resources and Docs + +[README](./README.md), a new artifact that includes guidelines and advice on when RFCs are appropriate and how to contribute them. +[0000 RFC Template](./0000-template.md), a new artifact that provides a clear starting point for proposals. +[Follow-up bug for updating CODEOWNERS](https://bugzilla.mozilla.org/show_bug.cgi?id=1881373), so that stakeholders can be found more easily. + +## Unresolved questions +- Will this result in an easier process for contributors to follow? +- Will this result in more engagement with RFCs? diff --git a/docs/rfcs/README.md b/docs/rfcs/README.md new file mode 100644 index 000000000000..dcccd71a6ea0 --- /dev/null +++ b/docs/rfcs/README.md @@ -0,0 +1,47 @@ +# The RFC Process + +An RFC (**R**equest **F**or **C**omments) is a process through which contributors can solicit buy-in +for proposed changes to the codebase and repository at-large. It was introduced in the first RFC, +[0001-rfc-process](./0001-rfc-process.md), which includes additional details about the reasoning +for including the process. + +This is an overview of what kind of changes benefit from or require the consensus-building that the +RFC process provides, as well as a brief guide on how to draft them. + +## What kinds of changes require an RFC? + +1. Substantial changes to public APIs in Android Components, like the changes found in [0003 Concept Base Component](./0003-concept-base-component.md) and [0008 Tab Groups](docs/rfcs/0008-tab-groups.md). +2. Changes to process that affect other teams, like the changes found in [0001 RFC Process](./0001-rfc-process.md), [0013 Add stakeholders to RFCs](./0013-rfc-process-updates.md), and [0007 Synchronized Releases](./0007-synchronized-releases.md). +3. Proposals for changes to areas of the codebase that are owned by CODEOWNERS outside the author's team. + +## What kind of other changes can an RFC be useful for? + +1. Announcing a rough plan for changes to a public API you own in order to solicit feedback. +2. Soliciting feedback for architectural changes that affect the entire codebase, like [0009 Remove Interactors and Controllers](./0009-remove-interactors-and-controllers.md). + +## How to contribute an RFC + +There is a [template](./0000-template.md) that can be a useful guide for structure. + +While drafting a proposal, consider the scope of your changes. Generally, the level of detail should match the level of +impact the changes will have on downstream consumers of APIs, other teams, or users. + +Once a proposal is drafted: + +1. Choose a deadline for general feedback. +2. Select and communicate with stakeholders. +3. Share the RFC more broadly through Slack and mailing lists for general feedback (like firefox-android-team@ and firefox-mobile@). +4. Build consensus and integrate feedback. + +### Stakeholders + +Stakeholders are required for each RFC. They will have the final say in acceptance and rejection. +Include at least 2 people as stakeholders: a CODEOWNER of the affected area and another (preferably a Firefox for Android team member). + +Stakeholders should be active in the RFC process - they should ask to be replaced if they do not have bandwidth to get the RFC finished in a short time span. This is to help the RFC process remain nimble and lightweight. + +### Deadlines + +A deadline for feedback should be included in each RFC. This should usually be at least a week, so plan accordingly. +For more substantial changes, it can be useful to plan for 2 or 3 weeks so that there is more opportunity for feedback from people that are not stakeholders. +If a proposal is approved by all stakeholders earlier than the deadline, the proposal can be merged immediately. \ No newline at end of file