Commit 38c016db authored by Azhara Assanova's avatar Azhara Assanova Committed by Commit Bot

[AW][Dev-UI] Add hide button for crashes in Crash UI

Create a field isHidden in CrashInfo class which is reflected in the
crash log json file. When the user presses "Hide crash" button, the
field is updated and the crash log json file is modified.
WebViewCrashInfoCollector's mergeDuplicates() also removes crashes that
have to be hidden.

Demo: https://drive.google.com/file/d/1EUlEzi4HXgzdgES0csWJpbVwaoiSJ9lU/view?usp=sharing
Dark theme: https://drive.google.com/file/d/1fmK5JStvNWv3ce17NUyQzSxTaUNyDdMB/view?usp=sharing

Bug: 1059944
Test: Manual - verify the hide button works as expected
Test: run_webview_instrumentation_test_apk -f DeveloperUiTest.*
Change-Id: Id08aab424a222c8acae2a22fcf1bb777bb2ac330
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339976
Commit-Queue: Azhara Assanova <azharaa@google.com>
Auto-Submit: Azhara Assanova <azharaa@google.com>
Reviewed-by: default avatarHazem Ashmawy <hazems@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarLaís Minchillo <laisminchillo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799139}
parent a87b7f44
...@@ -59,6 +59,12 @@ public class CrashInfo { ...@@ -59,6 +59,12 @@ public class CrashInfo {
*/ */
public long uploadTime = -1; public long uploadTime = -1;
/**
* Boolean that identifies if the crash should be hidden from the view or not.
* Default value is false.
*/
public boolean isHidden;
@NonNull @NonNull
private final Map<String, String> mCrashKeys; private final Map<String, String> mCrashKeys;
...@@ -66,6 +72,7 @@ public class CrashInfo { ...@@ -66,6 +72,7 @@ public class CrashInfo {
private static final String CRASH_CAPTURE_TIME_KEY = "crash-capture-time"; private static final String CRASH_CAPTURE_TIME_KEY = "crash-capture-time";
private static final String CRASH_UPLOAD_ID_KEY = "crash-upload-id"; private static final String CRASH_UPLOAD_ID_KEY = "crash-upload-id";
private static final String CRASH_UPLOAD_TIME_KEY = "crash-upload-time"; private static final String CRASH_UPLOAD_TIME_KEY = "crash-upload-time";
private static final String CRASH_IS_HIDDEN = "crash-is-hidden";
private static final String CRASH_KEYS_KEY = "crash-keys"; private static final String CRASH_KEYS_KEY = "crash-keys";
// Should match the crash keys used in minidump reports see for example of some keys: // Should match the crash keys used in minidump reports see for example of some keys:
// {@link android_webview/common/crash_reporter/crash_keys.cc} // {@link android_webview/common/crash_reporter/crash_keys.cc}
...@@ -172,6 +179,7 @@ public class CrashInfo { ...@@ -172,6 +179,7 @@ public class CrashInfo {
if (uploadTime != -1) { if (uploadTime != -1) {
jsonObj.put(CRASH_UPLOAD_TIME_KEY, uploadTime); jsonObj.put(CRASH_UPLOAD_TIME_KEY, uploadTime);
} }
jsonObj.put(CRASH_IS_HIDDEN, isHidden);
jsonObj.put(CRASH_KEYS_KEY, new JSONObject(mCrashKeys)); jsonObj.put(CRASH_KEYS_KEY, new JSONObject(mCrashKeys));
return jsonObj.toString(); return jsonObj.toString();
} catch (JSONException e) { } catch (JSONException e) {
...@@ -206,6 +214,10 @@ public class CrashInfo { ...@@ -206,6 +214,10 @@ public class CrashInfo {
crashInfo.uploadTime = jsonObj.getLong(CRASH_UPLOAD_TIME_KEY); crashInfo.uploadTime = jsonObj.getLong(CRASH_UPLOAD_TIME_KEY);
} }
if (jsonObj.has(CRASH_IS_HIDDEN)) {
crashInfo.isHidden = jsonObj.getBoolean(CRASH_IS_HIDDEN);
}
if (jsonObj.has(CRASH_KEYS_KEY)) { if (jsonObj.has(CRASH_KEYS_KEY)) {
JSONObject crashKeysObj = jsonObj.getJSONObject(CRASH_KEYS_KEY); JSONObject crashKeysObj = jsonObj.getJSONObject(CRASH_KEYS_KEY);
Iterator<String> keyIterator = crashKeysObj.keys(); Iterator<String> keyIterator = crashKeysObj.keys();
......
...@@ -103,6 +103,7 @@ public class CrashInfoTest { ...@@ -103,6 +103,7 @@ public class CrashInfoTest {
public void testSerializeToJson() throws Throwable { public void testSerializeToJson() throws Throwable {
final String jsonObjectString = final String jsonObjectString =
"{'crash-local-id':'123456abc','crash-capture-time':1234567890," "{'crash-local-id':'123456abc','crash-capture-time':1234567890,"
+ "'crash-is-hidden':false,"
+ "'crash-keys':{'app-package-name':'org.test.package'}}"; + "'crash-keys':{'app-package-name':'org.test.package'}}";
JSONObject expectedJsonObject = new JSONObject(jsonObjectString); JSONObject expectedJsonObject = new JSONObject(jsonObjectString);
......
...@@ -137,6 +137,7 @@ android_resources("devui_resources") { ...@@ -137,6 +137,7 @@ android_resources("devui_resources") {
"java/res_devui/drawable/ic_action_home.xml", "java/res_devui/drawable/ic_action_home.xml",
"java/res_devui/drawable/ic_alert_error.xml", "java/res_devui/drawable/ic_alert_error.xml",
"java/res_devui/drawable/ic_clear_text.xml", "java/res_devui/drawable/ic_clear_text.xml",
"java/res_devui/drawable/ic_delete.xml",
"java/res_devui/drawable/ic_flag.xml", "java/res_devui/drawable/ic_flag.xml",
"java/res_devui/drawable/ic_search.xml", "java/res_devui/drawable/ic_search.xml",
"java/res_devui/layout/activity_main.xml", "java/res_devui/layout/activity_main.xml",
......
<?xml version="1.0" encoding="utf-8"?>
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:pathData="M0 0h24v24H0z" />
<path
android:fillColor="@color/delete_button"
android:pathData="M6 19c0 1.1 0.9 2 2 2h8c1.1 0 2-0.9 2-2V7H6v12zM19 4h-3.5l-1-1h-5l-1 1H5v2h14V4z" />
</vector>
\ No newline at end of file
...@@ -19,9 +19,33 @@ ...@@ -19,9 +19,33 @@
android:paddingEnd="100dp" android:paddingEnd="100dp"
android:orientation="vertical"> android:orientation="vertical">
<include layout="@layout/two_line_sublist_item"
android:id="@+id/upload_status"/>
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="horizontal">
<LinearLayout
android:layout_width="0dp"
android:layout_weight=".70"
android:layout_height="wrap_content"
android:orientation="horizontal">
<include layout="@layout/two_line_sublist_item"
android:id="@+id/upload_status"/>
</LinearLayout>
<!--suppress HardcodedText -->
<ImageButton
android:contentDescription="Hide Crash Button"
android:id="@+id/crash_hide_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:paddingTop="4dp"
android:paddingBottom="4dp"
android:background="?android:selectableItemBackground"
android:src="@drawable/ic_delete" />
</LinearLayout>
<!--suppress HardcodedText --> <!--suppress HardcodedText -->
<Button <Button
android:id="@+id/crash_report_button" android:id="@+id/crash_report_button"
......
...@@ -13,21 +13,23 @@ ...@@ -13,21 +13,23 @@
android:mode="twoLine" android:mode="twoLine"
android:orientation="vertical"> android:orientation="vertical">
<TextView android:id="@android:id/text1" <TextView android:id="@android:id/text1"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:textAppearance="?android:attr/textAppearanceListItem" android:textAppearance="?android:attr/textAppearanceListItem"
android:textStyle="bold" android:textStyle="bold"
android:textAlignment="viewStart" android:textAlignment="viewStart"
android:paddingTop="4dp" android:paddingTop="4dp"
android:paddingBottom="4dp"/> android:paddingBottom="4dp"/>
<TextView android:id="@android:id/text2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="?android:attr/textAppearanceSmall"
android:textAlignment="viewStart"
android:paddingTop="4dp"
android:paddingBottom="4dp"/>
<TextView android:id="@android:id/text2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="?android:attr/textAppearanceSmall"
android:textAlignment="viewStart"
android:paddingTop="4dp"
android:paddingBottom="4dp"/>
</LinearLayout> </LinearLayout>
\ No newline at end of file
...@@ -9,4 +9,5 @@ ...@@ -9,4 +9,5 @@
<color name="error_red">#F28482</color> <color name="error_red">#F28482</color>
<color name="navigation_selected">#FFFFFF</color> <color name="navigation_selected">#FFFFFF</color>
<color name="medium_text_color">#E8EAED</color> <color name="medium_text_color">#E8EAED</color>
<color name="delete_button">#FFFFFF</color>
</resources> </resources>
...@@ -12,4 +12,5 @@ ...@@ -12,4 +12,5 @@
<color name="navigation_selected">#000000</color> <color name="navigation_selected">#000000</color>
<color name="navigation_unselected">#888888</color> <color name="navigation_unselected">#888888</color>
<color name="medium_text_color">#202124</color> <color name="medium_text_color">#202124</color>
<color name="delete_button">#000000</color>
</resources> </resources>
...@@ -24,6 +24,7 @@ import android.view.ViewGroup; ...@@ -24,6 +24,7 @@ import android.view.ViewGroup;
import android.widget.BaseExpandableListAdapter; import android.widget.BaseExpandableListAdapter;
import android.widget.Button; import android.widget.Button;
import android.widget.ExpandableListView; import android.widget.ExpandableListView;
import android.widget.ImageButton;
import android.widget.ImageView; import android.widget.ImageView;
import android.widget.TextView; import android.widget.TextView;
...@@ -212,7 +213,6 @@ public class CrashesListFragment extends DevUiBaseFragment { ...@@ -212,7 +213,6 @@ public class CrashesListFragment extends DevUiBaseFragment {
} }
setTwoLineListItemText(view.findViewById(R.id.crash_header), packageName, setTwoLineListItemText(view.findViewById(R.id.crash_header), packageName,
new Date(crashInfo.captureTime).toString()); new Date(crashInfo.captureTime).toString());
return view; return view;
} }
...@@ -296,6 +296,13 @@ public class CrashesListFragment extends DevUiBaseFragment { ...@@ -296,6 +296,13 @@ public class CrashesListFragment extends DevUiBaseFragment {
uploadButton.setVisibility(View.GONE); uploadButton.setVisibility(View.GONE);
} }
ImageButton hideButton = view.findViewById(R.id.crash_hide_button);
hideButton.setOnClickListener(v -> {
crashInfo.isHidden = true;
WebViewCrashInfoCollector.updateCrashLogFileWithNewCrashInfo(crashInfo);
updateCrashes();
});
return view; return view;
} }
......
...@@ -8,19 +8,27 @@ import androidx.annotation.VisibleForTesting; ...@@ -8,19 +8,27 @@ import androidx.annotation.VisibleForTesting;
import org.chromium.android_webview.common.crash.CrashInfo; import org.chromium.android_webview.common.crash.CrashInfo;
import org.chromium.android_webview.common.crash.SystemWideCrashDirectories; import org.chromium.android_webview.common.crash.SystemWideCrashDirectories;
import org.chromium.base.Log;
import org.chromium.components.minidump_uploader.CrashFileManager; import org.chromium.components.minidump_uploader.CrashFileManager;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashMap; import java.util.HashMap;
import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set;
/** /**
* Aggregates webview crash info from different sources into one single list. * Aggregates webview crash info from different sources into one single list.
* This list may be used to be displayed in a Crash UI. * This list may be used to be displayed in a Crash UI.
*/ */
public class WebViewCrashInfoCollector { public class WebViewCrashInfoCollector {
private static final String TAG = "WebViewCrashCollect";
private final CrashInfoLoader[] mCrashInfoLoaders; private final CrashInfoLoader[] mCrashInfoLoaders;
/** /**
...@@ -94,18 +102,31 @@ public class WebViewCrashInfoCollector { ...@@ -94,18 +102,31 @@ public class WebViewCrashInfoCollector {
/** /**
* Merge duplicate crashes (crashes which have the same local-id) into one object. * Merge duplicate crashes (crashes which have the same local-id) into one object.
* Removes crashes that have to be hidden.
*/ */
@VisibleForTesting @VisibleForTesting
public static List<CrashInfo> mergeDuplicates(List<CrashInfo> crashesList) { public static List<CrashInfo> mergeDuplicates(List<CrashInfo> crashesList) {
Map<String, CrashInfo> crashInfoMap = new HashMap<>(); Map<String, CrashInfo> crashInfoMap = new HashMap<>();
Set<String> hiddenCrashes = new HashSet<>();
for (CrashInfo c : crashesList) { for (CrashInfo c : crashesList) {
if (c.isHidden) {
hiddenCrashes.add(c.localId);
continue;
}
CrashInfo previous = crashInfoMap.get(c.localId); CrashInfo previous = crashInfoMap.get(c.localId);
if (previous != null) { if (previous != null) {
c = new CrashInfo(previous, c); c = new CrashInfo(previous, c);
} }
crashInfoMap.put(c.localId, c); crashInfoMap.put(c.localId, c);
} }
return new ArrayList<CrashInfo>(crashInfoMap.values());
List<CrashInfo> crashes = new ArrayList<>();
for (Map.Entry<String, CrashInfo> entry : crashInfoMap.entrySet()) {
if (!hiddenCrashes.contains(entry.getKey())) {
crashes.add(entry.getValue());
}
}
return crashes;
} }
/** /**
...@@ -120,4 +141,30 @@ public class WebViewCrashInfoCollector { ...@@ -120,4 +141,30 @@ public class WebViewCrashInfoCollector {
return 0; return 0;
}); });
} }
/**
* Modify WebView crash JSON log file with the new crash info if the JSON file exists
*/
public static void updateCrashLogFileWithNewCrashInfo(CrashInfo crashInfo) {
File logDir = SystemWideCrashDirectories.getWebViewCrashLogDir();
File[] logFiles = logDir.listFiles();
for (File logFile : logFiles) {
// Ignore non-json files.
if (!logFile.isFile() || !logFile.getName().endsWith(".json")) continue;
// Ignore unrelated json files
if (!logFile.getName().contains(crashInfo.localId)) continue;
try {
FileWriter writer = new FileWriter(logFile);
try {
writer.write(crashInfo.serializeToJson());
} finally {
writer.close();
}
} catch (IOException e) {
Log.e(TAG, "failed to modify JSON log entry for crash", e);
}
return;
}
// logfile does not exist
}
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment