[fenix] Fixed PR review

app/src/main/java/org/mozilla/fenix/perf/ProfilerStartDialogFragment.kt

@@ -105,7 +105,9 @@ class ProfilerStartDialogFragment : AppCompatDialogFragment() {
         viewStateObserver: MutableState<CardState>,
         startProfiler: (Array<String>, Array<String>) -> Unit
     ) {
-        val featureAndThreadsObserver = remember { mutableStateOf("") }
+        val featureAndThreadsObserver = remember {
+            mutableStateOf(requireContext().resources.getString(R.string.profiler_filter_firefox))
+        }
         ProfilerDialogueCard {
             Column(modifier = Modifier.padding(8.dp)) {
                 Text(

app/src/main/java/org/mozilla/fenix/perf/ProfilerStopDialogFragment.kt

@@ -71,10 +71,6 @@ class ProfilerStopDialogFragment : DialogFragment() {
                 // since the user needs to wait for the profiler data to be ready and we don't want to handle
                 // the process in the background.
                 if (viewStateObserver.value != CardState.WaitForProfilerState) {
-                    profilerViewModel.setProfilerState(
-                        requireContext()
-                            .components.core.engine.profiler!!.isProfilerActive()
-                    )
                     this@ProfilerStopDialogFragment.dismiss()
                 }
             }
@@ -116,7 +112,8 @@ class ProfilerStopDialogFragment : DialogFragment() {
                 ) {
                     TextButton(
                         onClick = {
-                            displayToastAndDismiss(R.string.profile_stop_dialogue_cancel_save)
+                            requireContext().components.core.engine.profiler?.stopProfiler({}, {})
+                            dismiss()
                         }
                     ) {
                         Text(stringResource(R.string.profiler_start_cancel))
@@ -151,9 +148,9 @@ class ProfilerStopDialogFragment : DialogFragment() {
             },
             onError = {
                 error ->
-                error.message?.let {
+                if (error.message != null) {
                     displayToastAndDismiss(R.string.profiler_error, " error: $error")
-                }?.let {
+                } else {
                     displayToastAndDismiss(R.string.profiler_error)
                 }
             }

app/src/main/java/org/mozilla/fenix/perf/ProfilerUtils.kt

@@ -74,14 +74,18 @@ enum class ProfilerSettings(val threads: Array<String>, val features: Array<Stri
  */
 object ProfilerUtils {
 
-    private fun saveProfileUrlToClipboardAndToast(profileResult: ByteArray, context: Context): String {
+    private fun saveProfileUrlToClipboard(profileResult: ByteArray, context: Context): String {
         // The profile is saved to a temporary file since our fetch API takes a file or a string.
         // Converting the ByteArray to a String would hurt the encoding, which we need to preserve.
+        // The file may potentially contain sensitive data and should be handled carefully.
         val outputFile = createTemporaryFile(profileResult, context)
-        val response = networkCallToProfilerServer(outputFile, context)
-        val profileToken = decodeProfileToken(response)
-        outputFile.delete()
-        return PROFILER_DATA_URL + profileToken
+        try {
+            val response = networkCallToProfilerServer(outputFile, context)
+            val profileToken = decodeProfileToken(response)
+            return PROFILER_DATA_URL + profileToken
+        } finally {
+            outputFile.delete()
+        }
     }
 
     private fun finishProfileSave(context: Context, url: String, onUrlFinish: (Int) -> Unit) {
@@ -138,7 +142,7 @@ object ProfilerUtils {
         onUrlFinish: (Int) -> Unit
     ) {
         try {
-            val url = saveProfileUrlToClipboardAndToast(profile, context)
+            val url = saveProfileUrlToClipboard(profile, context)
             finishProfileSave(context, url, onUrlFinish)
         } catch (e: IOException) {
             onUrlFinish(R.string.profiler_io_error)

app/src/main/java/org/mozilla/fenix/perf/ProfilerViewModel.kt

@@ -4,20 +4,32 @@
 
 package org.mozilla.fenix.perf
 
+import android.app.Application
+import androidx.lifecycle.AndroidViewModel
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.ViewModel
+import org.mozilla.fenix.FenixApplication
+import org.mozilla.fenix.ext.components
 
 /**
  * [ViewModel] to keep track of the profiler state
  */
-class ProfilerViewModel : ViewModel() {
-    var isProfilerActive: MutableLiveData<Boolean> = MutableLiveData()
+class ProfilerViewModel(application: Application) : AndroidViewModel(application) {
+    val isProfilerActive: MutableLiveData<Boolean> = MutableLiveData(
+        application.components.core.engine.profiler?.isProfilerActive() ?: false
+    )
 
     /**
      * @return profiler status
      */
     fun getProfilerState(): LiveData<Boolean> {
+        // We check here since this can be polled from anywhere in Fenix.
+        getApplication<FenixApplication>().components.core.engine.profiler?.let {
+            check(it.isProfilerActive() == isProfilerActive.value) {
+                "The Profiler state from Gecko is out of sync with the LiveData profiler state."
+            }
+        }
         return isProfilerActive
     }
 

app/src/main/java/org/mozilla/fenix/settings/SettingsFragment.kt

@@ -419,7 +419,7 @@ class SettingsFragment : PreferenceFragmentCompat() {
                 null
             }
             resources.getString(R.string.pref_key_start_profiler) -> {
-                if (requireContext().components.core.engine.profiler?.isProfilerActive()!!) {
+                if (profilerViewModel.getProfilerState().value == true) {
                     SettingsFragmentDirections.actionSettingsFragmentToStopProfilerDialog()
                 } else {
                     SettingsFragmentDirections.actionSettingsFragmentToStartProfilerDialog()

app/src/main/res/values/static_strings.xml

@@ -57,6 +57,7 @@
     <!-- Profiler settings -->
     <string name="preferences_start_profiler">Start Profiler</string>
     <string name="profiler_stop">Stop Profiler</string>
+
     <string name="profiler_settings_title">Profiler Settings</string>
     <string name="profiler_filter_firefox">Firefox</string>
     <string name="profiler_running">Profiler is currently running</string>
@@ -67,30 +68,21 @@
     <string name="profiler_filter_media_explain">Preset for investigating audio and video bugs in Firefox</string>
     <string name="profiler_filter_networking">Networking</string>
     <string name="profiler_filter_networking_explain">Preset for investigating networking bugs in Firefox</string>
+
     <string name="profiler_start_dialog_started">Profiler started</string>
-    <string name="profiler_save_method">Would you like to save your profile as:</string>
-    <string name="profiler_save_method_url">URL</string>
-    <string name="profiler_save_method_url_explained">The profile will upload automatically with all the information gathered and provide you a link from profiler.firefox.com</string>
-    <string name="profiler_save_method_file">File</string>
-    <string name="profiler_save_method_file_explained">The file will be saved locally on your android device. To share it, go to profiler.firefox.com on your computer or mobile device</string>
+
     <string name="profiler_start_cancel">Cancel</string>
+
     <string name="profiler_gathering">Gathering the profile</string>
-    <string name="profiler_waiting_start">Waiting for Profiler to start</string>
-    <string name="profiler_url_warning">Saving to URL warning</string>
-    <string name="profiler_url_warning_explained">"Saving to URL could potentially show private information such as " +
-                        "urls and screenshots. If you want to keep these" +
-                        "privates, save your profile as a file instead and head on over to profiler.firefox.com" +
-                        "where you can select what to share and not share."</string>
-    <string name="profiler_file_save">Save as File</string>
-    <string name="profiler_as_url">Get Profile URL</string>
     <string name="profiler_no_info">No information was gathered</string>
+
+    <string name="profiler_waiting_start">Waiting for Profiler to start</string>
+
+    <string name="profiler_url_warning">⚠️ Warning: Profile Upload</string>
+    <string name="profiler_url_warning_explained">Performance profiles may contain sensitive information such as websites visited and screenshots of them. If you continue, your profile will be uploaded and made accessible to anyone with the link. Do you wish to continue?</string>
+    <string name="profiler_as_url">Upload profile</string>
     <string name="profiler_error">Something went wrong with the profiler</string>
     <string name="profiler_io_error">Something went wrong contacting the Profiler server.</string>
-    <string name="profiler_file_error">Something went wrong writing the profile data to the file</string>
-    <string name="profiler_error_saving">Something went wrong saving the profile</string>
 
-    <string name="profiler_stop_dialogue_file_saved">The profile was saved locally</string>
-    <string name="profiler_stop_dialogue_file_saved_failed">Saving profile to file failed, please try again</string>
     <string name="profiler_uploaded_url_to_clipboard">URL copied to clipboard successfully</string>
-    <string name="profile_stop_dialogue_cancel_save">Profile save operation canceled</string>
 </resources>

app/src/test/java/org/mozilla/fenix/settings/SettingsFragmentTest.kt

@@ -42,15 +42,16 @@ class SettingsFragmentTest {
 
     @Before
     fun setup() {
+
         // Mock client for fetching account avatar
         val client = mockk<Client>()
         every { client.fetch(any()) } throws IOException("test")
 
+        every { testContext.components.core.engine.profiler } returns mockk(relaxed = true)
         every { testContext.components.core.client } returns client
         every { testContext.components.settings } returns mockk(relaxed = true)
         every { testContext.components.analytics } returns mockk(relaxed = true)
         every { testContext.components.backgroundServices } returns mockk(relaxed = true)
-
         mockkObject(Config)
         every { Config.channel } returns ReleaseChannel.Nightly