Skip to content

GitLab

T
tangled
Commit 4d006cbf authored by Hazem Ashmawy's avatar Hazem Ashmawy Committed by Commit Bot
Browse files
Options

[AW] DevUI: Polish WebView crash UI 

Polish WebView crash UI layout:
- Use android material theme and support dark theme.
- Use an expandable list to show crashes where we show the important
  info (package name and crash capture time in the header) and show
  variation keys, upload status, report button in the expanded body.
- Use a more descriptive string for the reporting button
  "File bug report" instead of "Provide more info".
- For better visualization, show app icon next to package name to
  be easy to find by eye.
- Use ListViewAdapter#notifyDataSetChanged() instead of creating
  new adapter to optimize crash list updates.
- Update MainActivity to use the new two_line_list_item layout.

Bug: 948923
Test: Manually inspect the UI on SystemWebView on Android P and Android Q with dark mode switched on/off.
Change-Id: Ic9d4fffc6f24a409f06927b91b160280c5781e7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1899371
Commit-Queue: Hazem Ashmawy <hazems@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarNate Fischer <ntfschr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713623}
parent b0a9322a
Hide whitespace changes
Inline Side-by-side
Showing with 343 additions and 94 deletions
android_webview/apk/BUILD.gn
View file @ 4d006cbf
......@@ -52,6 +52,7 @@ android_library("devui_java") {
"//android_webview:common_crash_java",
"//base:base_java",
"//components/minidump_uploader:minidump_uploader_java",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//ui/android:ui_java",
]
android_manifest_for_lint = system_webview_android_manifest
......
android_webview/apk/java/AndroidManifest.xml
View file @ 4d006cbf
......@@ -33,7 +33,7 @@
<!--suppress HardcodedText -->
<activity android:name="org.chromium.android_webview.devui.MainActivity"
android:label="WebView DevTools"
android:theme="@android:style/Theme.DeviceDefault"
android:theme="@style/Theme.DevUi.DayNight"
android:launchMode="singleTask"
android:process=":webview_apk"> {# Explicit process required for monochrome compatibility. #}
<intent-filter>
......@@ -43,7 +43,7 @@
</activity>
<activity android:name="org.chromium.android_webview.devui.CrashesListActivity"
android:label="WebView Crashes"
android:theme="@android:style/Theme.DeviceDefault"
android:theme="@style/Theme.DevUi.DayNight"
android:launchMode="singleTop"
android:process=":webview_apk"> {# Explicit process required for monochrome compatibility. #}
</activity>
......
android_webview/apk/java/res_devui/layout/activity_crashes_list.xml
View file @ 4d006cbf
......@@ -7,7 +7,6 @@
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/activity_crashes_list"
android:orientation="vertical"
android:layout_width="match_parent"
android:layout_height="match_parent">
......@@ -16,16 +15,23 @@
android:id="@+id/crashes_summary_textview"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:background="#A9A9A9"
android:textColor="#FFFFFF"
android:textSize="20sp"
android:paddingBottom="5sp"
android:paddingTop="5sp"/>
android:paddingTop="16dp"
android:paddingBottom="16dp"
android:paddingStart="10dp"
android:paddingEnd="10dp"
android:textAppearance="?android:attr/textAppearanceLarge"/>
<ListView
<!-- horizontal divider -->
<View
android:layout_width="match_parent"
android:layout_height="1dp"
android:background="?android:attr/listDivider"/>
<!-- child divider is transparent (hidden) -->
<ExpandableListView
android:id="@+id/crashes_list"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:layout_marginTop="25dp"/>
android:childDivider="#00000000"/>
</LinearLayout>
android_webview/apk/java/res_devui/layout/activity_main.xml
View file @ 4d006cbf
......@@ -7,7 +7,6 @@
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/activity_main"
android:orientation="vertical"
android:layout_width="match_parent"
android:layout_height="match_parent">
......
android_webview/apk/java/res_devui/layout/crashes_list_item_body.xml 0 → 100644
View file @ 4d006cbf
<?xml version="1.0" encoding="utf-8"?>
<!--
Copyright 2019 The Chromium Authors. All rights reserved. Use of this
source code is governed by a BSD-style license that can be found in the
LICENSE file.
-->
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingTop="2dp"
android:paddingBottom="2dp"
android:orientation="vertical">
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingStart="?android:attr/expandableListPreferredItemPaddingLeft"
android:paddingEnd="100dp"
android:orientation="vertical">
<include layout="@layout/two_line_sublist_item"
android:id="@+id/variations"/>
<include layout="@layout/two_line_sublist_item"
android:id="@+id/upload_status"/>
<!--suppress HardcodedText -->
<Button
android:id="@+id/crash_report_button"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingTop="8dp"
android:paddingBottom="8dp"
android:text="File bug report"
android:textAllCaps="false" />
</LinearLayout>
<!-- horizontal divider -->
<View
android:layout_width="match_parent"
android:layout_height="1dp"
android:layout_marginTop="16dp"
android:background="?android:attr/listDivider"/>
</LinearLayout>
\ No newline at end of file
android_webview/apk/java/res_devui/layout/crashes_list_item_header.xml 0 → 100644
View file @ 4d006cbf
<?xml version="1.0" encoding="utf-8"?>
<!--
Copyright 2019 The Chromium Authors. All rights reserved. Use of this
source code is governed by a BSD-style license that can be found in the
LICENSE file.
-->
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingStart="?android:attr/expandableListPreferredItemPaddingLeft"
android:paddingTop="16dp"
android:paddingBottom="16dp"
android:gravity="center"
android:orientation="horizontal">
<!--suppress HardcodedText -->
<ImageView
android:layout_height="35dp"
android:layout_width="35dp"
android:id="@+id/crash_package_icon"
android:layout_marginEnd="2dp"
android:contentDescription="package icon"/>
<include layout="@layout/two_line_list_item"
android:id="@+id/crash_header"/>
</LinearLayout>
\ No newline at end of file
android_webview/apk/java/res_devui/layout/crashes_list_row.xml → android_webview/apk/java/res_devui/layout/two_line_list_item.xml
View file @ 4d006cbf
......@@ -5,37 +5,31 @@
LICENSE file.
-->
<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/crash_list_row"
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="15dp"
android:paddingTop="2dp"
android:paddingBottom="2dp"
android:paddingStart="?android:attr/listPreferredItemPaddingStart"
android:paddingEnd="?android:attr/listPreferredItemPaddingEnd"
android:mode="twoLine"
android:orientation="vertical">
<TextView
android:id="@+id/crash_info_label"
<TextView android:id="@android:id/text1"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textIsSelectable="true"
android:textAppearance="?android:attr/textAppearanceMedium"
android:textStyle="bold"
android:textSize="16sp"
android:paddingBottom="2dp"
android:paddingTop="2dp"
android:background="#d3d3d3"/>
android:textAlignment="viewStart"
android:paddingTop="4dp"
android:paddingBottom="4dp"/>
<TextView
android:id="@+id/crash_info_textview"
<TextView android:id="@android:id/text2"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="5dp"
android:textIsSelectable="true"/>
android:textAppearance="?android:attr/textAppearanceSmall"
android:textAlignment="viewStart"
android:paddingTop="4dp"
android:paddingBottom="4dp"/>
<!--suppress HardcodedText -->
<Button
android:id="@+id/crash_report_button"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:text="Provide more info"/>
</LinearLayout>
</LinearLayout>
\ No newline at end of file
android_webview/apk/java/res_devui/layout/two_line_sublist_item.xml 0 → 100644
View file @ 4d006cbf
<?xml version="1.0" encoding="utf-8"?>
<!--
Copyright 2019 The Chromium Authors. All rights reserved. Use of this
source code is governed by a BSD-style license that can be found in the
LICENSE file.
-->
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:paddingStart="2dp"
android:paddingEnd="2dp"
android:mode="twoLine"
android:orientation="vertical">
<TextView android:id="@android:id/text1"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="?android:attr/textAppearanceListItem"
android:textStyle="bold"
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>
\ No newline at end of file
android_webview/apk/java/res_devui/menu/crashes_options_menu.xml
View file @ 4d006cbf
......@@ -5,8 +5,7 @@
LICENSE file.
-->
<menu xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto">
<menu xmlns:android="http://schemas.android.com/apk/res/android">
<group android:id="@+id/crashes_menu">
<!--suppress HardcodedText -->
<item
......
android_webview/apk/java/res_devui/values-night/styles.xml 0 → 100644
View file @ 4d006cbf
<?xml version="1.0" encoding="utf-8"?>
<resources>
<style name="Theme.DevUi.DayNight" parent="@android:style/Theme.Material" />
</resources>
\ No newline at end of file
android_webview/apk/java/res_devui/values/styles.xml 0 → 100644
View file @ 4d006cbf
<?xml version="1.0" encoding="utf-8"?>
<resources>
<style name="Theme.DevUi.DayNight" parent="@android:style/Theme.Material.Light.DarkActionBar" />
</resources>
\ No newline at end of file
android_webview/apk/java/src/org/chromium/android_webview/devui/CrashesListActivity.java
View file @ 4d006cbf
......@@ -6,19 +6,23 @@ package org.chromium.android_webview.devui;
import android.app.Activity;
import android.content.Intent;
import android.content.pm.PackageManager;
import android.graphics.drawable.Drawable;
import android.net.Uri;
import android.os.Bundle;
import android.text.SpannableStringBuilder;
import android.text.style.StyleSpan;
import android.view.Menu;
import android.view.MenuItem;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ArrayAdapter;
import android.widget.BaseExpandableListAdapter;
import android.widget.Button;
import android.widget.ListView;
import android.widget.ExpandableListView;
import android.widget.ImageView;
import android.widget.TextView;
import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import org.chromium.android_webview.common.crash.CrashInfo;
import org.chromium.android_webview.common.crash.CrashInfo.UploadState;
import org.chromium.android_webview.devui.util.NavigationMenuHelper;
......@@ -35,10 +39,12 @@ public class CrashesListActivity extends Activity {
// Max number of crashes to show in the crashes list.
private static final int MAX_CRASHES_NUMBER = 20;
private ListView mCrashListView;
private TextView mCrashesSummaryView;
private BaseExpandableListAdapter mCrashListViewAdapter;
private WebViewCrashInfoCollector mCrashCollector;
private List<CrashInfo> mCrashInfoList;
private static final String CRASH_REPORT_TEMPLATE = ""
+ "IMPORTANT: Your crash has already been automatically reported to our crash system. "
+ "You only need to fill this out if you can share more information like steps to "
......@@ -76,40 +82,87 @@ public class CrashesListActivity extends Activity {
setContentView(R.layout.activity_crashes_list);
mCrashListView = findViewById(R.id.crashes_list);
mCrashesSummaryView = findViewById(R.id.crashes_summary_textview);
mCrashCollector = new WebViewCrashInfoCollector();
mCrashListViewAdapter = new CrashListExpandableAdapter();
// initialize the crash list before setting the list adapter.
updateCrashesList();
ExpandableListView crashListView = findViewById(R.id.crashes_list);
crashListView.setAdapter(mCrashListViewAdapter);
}
/**
* Adapter to create crashes list rows from a list of CrashInfo.
* Adapter to create crashes list items from a list of CrashInfo.
*/
private class CrashListAdapter extends ArrayAdapter<CrashInfo> {
private final List<CrashInfo> mCrashInfoList;
private class CrashListExpandableAdapter extends BaseExpandableListAdapter {
// Group View which is used as header for a crash in crashes list.
// We show:
// - Icon of the app where the crash happened.
// - Package name of the app where the crash happened.
// - Time when the crash happened.
@Override
public View getGroupView(
int groupPosition, boolean isExpanded, View view, ViewGroup parent) {
// If the the old view is already created then reuse it, else create a new one by layout
// inflation.
if (view == null) {
view = getLayoutInflater().inflate(R.layout.crashes_list_item_header, null);
}
CrashInfo crashInfo = (CrashInfo) getGroup(groupPosition);
public CrashListAdapter(List<CrashInfo> crashInfoList) {
super(CrashesListActivity.this, R.layout.crashes_list_row, crashInfoList);
mCrashInfoList = crashInfoList;
ImageView packageIcon = view.findViewById(R.id.crash_package_icon);
String packageName;
if (crashInfo.packageName == null) {
// This can happen if crash log file where we keep crash info is cleared but other
// log files like upload logs still exist.
packageName = "unknown app";
packageIcon.setImageResource(android.R.drawable.sym_def_app_icon);
} else {
packageName = crashInfo.packageName;
try {
Drawable icon = getPackageManager().getApplicationIcon(packageName);
packageIcon.setImageDrawable(icon);
} catch (PackageManager.NameNotFoundException e) {
// This can happen if the app was uninstalled after the crash was recorded.
packageIcon.setImageResource(android.R.drawable.sym_def_app_icon);
}
}
setTwoLineListItemText(view.findViewById(R.id.crash_header), packageName,
new Date(crashInfo.captureTime).toString());
return view;
}
// Child View where more info about the crash is shown:
// - Variation keys for the crash.
// - Crash report upload status.
@Override
public View getView(int position, View view, ViewGroup parent) {
public View getChildView(int groupPosition, final int childPosition, boolean isLastChild,
View view, ViewGroup parent) {
// If the the old view is already created then reuse it, else create a new one by layout
// inflation.
if (view == null) {
view = getLayoutInflater().inflate(R.layout.crashes_list_row, null, true);
view = getLayoutInflater().inflate(R.layout.crashes_list_item_body, null);
}
TextView label = view.findViewById(R.id.crash_info_label);
TextView infoTextView = view.findViewById(R.id.crash_info_textview);
Button button = view.findViewById(R.id.crash_report_button);
CrashInfo crashInfo = mCrashInfoList.get(position);
label.setText(getCrashInfoLabelText(crashInfo));
infoTextView.setText(crashInfoToString(crashInfo));
CrashInfo crashInfo = (CrashInfo) getChild(groupPosition, childPosition);
// Variations keys
setTwoLineListItemText(view.findViewById(R.id.variations), "Variations",
crashInfo.variations == null ? "Not available"
: crashInfo.variations.toString());
// Upload info
String uploadState = uploadStateString(crashInfo.uploadState);
String uploadInfo = null;
if (crashInfo.uploadState == UploadState.UPLOADED) {
uploadInfo = new Date(crashInfo.uploadTime).toString();
uploadInfo += "\nID: " + crashInfo.uploadId;
}
setTwoLineListItemText(view.findViewById(R.id.upload_status), uploadState, uploadInfo);
Button button = view.findViewById(R.id.crash_report_button);
// Report button is only clickable if the crash report is uploaded.
if (crashInfo.uploadState == UploadState.UPLOADED) {
button.setEnabled(true);
......@@ -123,70 +176,110 @@ public class CrashesListActivity extends Activity {
return view;
}
private String getCrashInfoLabelText(CrashInfo crashInfo) {
String status =
crashInfo.uploadState == null ? "UNKOWN" : crashInfo.uploadState.toString();
return String.format(Locale.US, "Crash (%s) - [%s]", crashInfo.localId, status);
@Override
public boolean isChildSelectable(int groupPosition, int childPosition) {
return true;
}
/**
* Convert CrashInfo object to a string to show it in a TexView.
* Field name is in BOLD while the value is not.
*/
private SpannableStringBuilder crashInfoToString(CrashInfo crashInfo) {
SpannableStringBuilder builder = new SpannableStringBuilder();
if (crashInfo.uploadId != null) {
builder.append("upload ID: ", new StyleSpan(android.graphics.Typeface.BOLD), 0)
.append(crashInfo.uploadId)
.append("\n");
}
if (crashInfo.uploadTime >= 0) {
builder.append("upload time: ", new StyleSpan(android.graphics.Typeface.BOLD), 0)
.append(new Date(crashInfo.uploadTime).toString())
.append("\n");
}
if (crashInfo.captureTime >= 0) {
builder.append("capture time: ", new StyleSpan(android.graphics.Typeface.BOLD), 0)
.append(new Date(crashInfo.captureTime).toString())
.append("\n");
}
if (crashInfo.packageName != null) {
builder.append("app package name: ", new StyleSpan(android.graphics.Typeface.BOLD),
0)
.append(crashInfo.packageName)
.append("\n");
}
if (crashInfo.variations != null) {
builder.append("variations: ", new StyleSpan(android.graphics.Typeface.BOLD), 0)
.append(crashInfo.variations.toString())
.append("\n");
}
return builder;
@Override
public Object getGroup(int groupPosition) {
return mCrashInfoList.get(groupPosition);
}
// Build a report uri to open an issue on https://bugs.chromium.org/p/chromium/issues/entry.
// It uses WebView Bugs Template and adds "User-Submitted" Label.
// It adds the upload id at the end of the template and populates the Application package
// name field.
// TODO(https://crbug.com/991594) populate more fields in the template.
private Uri getReportUri(CrashInfo crashInfo) {
return new Uri.Builder()
.scheme("https")
.authority("bugs.chromium.org")
.path("/p/chromium/issues/entry")
.appendQueryParameter("template", "Webview+Bugs")
.appendQueryParameter("comment",
String.format(Locale.US, CRASH_REPORT_TEMPLATE, crashInfo.uploadId))
.appendQueryParameter("labels", "User-Submitted")
.build();
@Override
public Object getChild(int groupPosition, int childPosition) {
return mCrashInfoList.get(groupPosition);
}
@Override
public long getGroupId(int groupPosition) {
// Hash code of local id is unique per crash info object.
return ((CrashInfo) getGroup(groupPosition)).localId.hashCode();
}
@Override
public long getChildId(int groupPosition, int childPosition) {
// Child ID refers to a piece of info in a particular crash report. It is stable
// since we don't change the order we show this info in runtime and currently we show
// all information in only one child item.
return childPosition;
}
@Override
public boolean hasStableIds() {
// Stable IDs mean both getGroupId and getChildId return stable IDs for each group and
// child i.e: an ID always refers to the same object. See getGroupId and getChildId for
// why the IDs are stable.
return true;
}
@Override
public int getChildrenCount(int groupPosition) {
// Crash info is shown in one child item.
return 1;
}
@Override
public int getGroupCount() {
return mCrashInfoList.size();
}
}
private void updateCrashesList() {
List<CrashInfo> crashesList = mCrashCollector.loadCrashesInfo(MAX_CRASHES_NUMBER);
mCrashListView.setAdapter(new CrashListAdapter(crashesList));
// Build a report uri to open an issue on https://bugs.chromium.org/p/chromium/issues/entry.
// It uses WebView Bugs Template and adds "User-Submitted" Label.
// It adds the upload id at the end of the template and populates the Application package
// name field.
// TODO(https://crbug.com/991594) populate more fields in the template.
private static Uri getReportUri(CrashInfo crashInfo) {
return new Uri.Builder()
.scheme("https")
.authority("bugs.chromium.org")
.path("/p/chromium/issues/entry")
.appendQueryParameter("template", "Webview+Bugs")
.appendQueryParameter("comment",
String.format(Locale.US, CRASH_REPORT_TEMPLATE, crashInfo.uploadId))
.appendQueryParameter("labels", "User-Submitted")
.build();
}
mCrashesSummaryView.setText(String.format(Locale.US, "Crashes (%d)", crashesList.size()));
private static String uploadStateString(UploadState uploadState) {
switch (uploadState) {
case UPLOADED:
return "Uploaded";
case PENDING:
case PENDING_USER_REQUESTED:
return "Pending upload";
case SKIPPED:
return "Skipped upload";
}
return null;
}
// Helper method to find and set text for two line list item. If a null String is passed, the
// relevant TextView will be hidden.
private static void setTwoLineListItemText(
@NonNull View view, @Nullable String title, @Nullable String subtitle) {
TextView titleView = view.findViewById(android.R.id.text1);
TextView subtitleView = view.findViewById(android.R.id.text2);
if (titleView != null) {
titleView.setVisibility(View.VISIBLE);
titleView.setText(title);
} else {
titleView.setVisibility(View.GONE);
}
if (subtitle != null) {
subtitleView.setVisibility(View.VISIBLE);
subtitleView.setText(subtitle);
} else {
subtitleView.setVisibility(View.GONE);
}
}
private void updateCrashesList() {
mCrashInfoList = mCrashCollector.loadCrashesInfo(MAX_CRASHES_NUMBER);
mCrashListViewAdapter.notifyDataSetChanged();
mCrashesSummaryView.setText(
String.format(Locale.US, "Crashes (%d)", mCrashInfoList.size()));
}
@Override
......
android_webview/apk/java/src/org/chromium/android_webview/devui/MainActivity.java
View file @ 4d006cbf
......@@ -94,7 +94,7 @@ public class MainActivity extends Activity {
private final InfoItem[] mItems;
public InfoListAdapter(InfoItem[] items) {
super(MainActivity.this, android.R.layout.simple_list_item_2, items);
super(MainActivity.this, R.layout.two_line_list_item, items);
mItems = items;
}
......@@ -103,7 +103,7 @@ public class MainActivity extends Activity {
// If the the old view is already created then reuse it, else create a new one by layout
// inflation.
if (view == null) {
view = getLayoutInflater().inflate(android.R.layout.simple_list_item_2, null, true);
view = getLayoutInflater().inflate(R.layout.two_line_list_item, null, true);
}
InfoItem item = mItems[position];
......
chrome/android/java/monochrome_public_bundle__base_bundle_module.AndroidManifest.expected
View file @ 4d006cbf
......@@ -1012,14 +1012,14 @@
android:launchMode="singleTop"
android:name="org.chromium.android_webview.devui.CrashesListActivity"
android:process=":webview_apk"
android:theme="@android:style/Theme.DeviceDefault"/>
android:theme="@style/Theme.DevUi.DayNight"/>
<activity
android:label="WebView
DevTools"
android:launchMode="singleTask"
android:name="org.chromium.android_webview.devui.MainActivity"
android:process=":webview_apk"
android:theme="@android:style/Theme.DeviceDefault">
android:theme="@style/Theme.DevUi.DayNight">
<intent-filter>
<action android:name="com.android.webview.SHOW_DEV_UI"/>
<category android:name="android.intent.category.DEFAULT"/>
......
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