Commit 31da6c37 authored by Torne (Richard Coles)'s avatar Torne (Richard Coles) Committed by Commit Bot

Remove most AwDebug functionality.

AwDebug.dumpWithoutCrashing was added to diagnose a suspected renderer
hang; it's only accessible by reflection and has not been used to debug
an issue for a long time. It no longer provides very useful output since
it only dumps the current process and WebView is now multiprocess in the
majority of cases.

Therefore, just disable the API entirely rather than making it more
complex to support multiprocess - if there's a similar need in future it
would be more useful to support capturing a trace than a minidump, which
did not prove valuable. We keep it around so that any hypothetical
clients won't see exceptions during reflection (though they should be
handling those exceptions) - it just always returns false to indicate
the dump was not generated.

AwDebugTest relied on dumpWithoutCrashing to test the crash key
filtering mechanism, but the tests have been disabled for some time and
maintaining this just for a test doesn't seem worth it; just remove
those disabled tests entirely.

Fixed: 568825
Fixed: 913515
Change-Id: I2b5b242388d6aaac14508f554e58c3eabbc4ae5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2231658Reviewed-by: default avatarBo <boliu@chromium.org>
Reviewed-by: default avatarChangwan Ryu <changwan@chromium.org>
Commit-Queue: Richard Coles <torne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775297}
parent 80574b5c
......@@ -2,187 +2,14 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include <string.h>
#include <memory>
#include "android_webview/browser_jni_headers/AwDebug_jni.h"
#include "android_webview/common/crash_reporter/aw_crash_reporter_client.h"
#include "android_webview/common/crash_reporter/crash_keys.h"
#include "base/android/jni_android.h"
#include "base/android/jni_string.h"
#include "base/android/path_utils.h"
#include "base/debug/dump_without_crashing.h"
#include "base/files/file.h"
#include "base/files/file_path.h"
#include "base/files/file_util.h"
#include "base/threading/thread_restrictions.h"
#include "components/crash/core/app/crash_reporter_client.h"
#include "components/crash/core/app/crashpad.h"
#include "components/crash/core/common/crash_key.h"
#include "components/minidump_uploader/rewrite_minidumps_as_mimes.h"
#include "components/version_info/android/channel_getter.h"
#include "components/version_info/version_info.h"
#include "components/version_info/version_info_values.h"
#include "third_party/crashpad/crashpad/client/crash_report_database.h"
#include "third_party/crashpad/crashpad/util/net/http_body.h"
#include "third_party/crashpad/crashpad/util/net/http_multipart_builder.h"
using base::android::ConvertJavaStringToUTF16;
using base::android::ConvertUTF8ToJavaString;
using base::android::JavaParamRef;
using base::android::ScopedJavaLocalRef;
namespace android_webview {
namespace {
class AwDebugCrashReporterClient
: public ::crash_reporter::CrashReporterClient {
public:
AwDebugCrashReporterClient() = default;
~AwDebugCrashReporterClient() override = default;
void GetProductNameAndVersion(std::string* product_name,
std::string* version,
std::string* channel) override {
*product_name = "AndroidWebView";
*version = PRODUCT_VERSION;
*channel =
version_info::GetChannelString(version_info::android::GetChannel());
}
bool GetCrashDumpLocation(base::FilePath* debug_dir) override {
base::FilePath cache_dir;
if (!base::android::GetCacheDirectory(&cache_dir)) {
return false;
}
*debug_dir = cache_dir.Append(FILE_PATH_LITERAL("WebView")).Append("Debug");
return true;
}
void GetSanitizationInformation(const char* const** crash_key_allowlist,
void** target_module,
bool* sanitize_stacks) override {
*crash_key_allowlist = crash_keys::kWebViewCrashKeyAllowList;
*target_module = nullptr;
*sanitize_stacks = true;
}
DISALLOW_COPY_AND_ASSIGN(AwDebugCrashReporterClient);
};
// Writes the most recent report in the database as a MIME to fd. Finishes by
// deleting all reports from the database. Returns `true` if a report was
// successfully found and written the file descriptor.
bool WriteLastReportToFd(crashpad::CrashReportDatabase* db, int fd) {
std::vector<crashpad::CrashReportDatabase::Report> reports;
if (db->GetPendingReports(&reports) !=
crashpad::CrashReportDatabase::kNoError ||
!reports.size()) {
LOG(ERROR) << "no reports";
return false;
}
size_t most_recent_index = 0;
time_t most_recent_time = reports[0].creation_time;
for (size_t index = 1; index < reports.size(); ++index) {
if (reports[index].creation_time > most_recent_time) {
most_recent_index = index;
most_recent_time = reports[index].creation_time;
}
}
{
std::unique_ptr<const crashpad::CrashReportDatabase::UploadReport>
upload_report;
if (db->GetReportForUploading(reports[most_recent_index].uuid,
&upload_report,
/* report_metrics= */ false) !=
crashpad::CrashReportDatabase::kNoError) {
return false;
}
crashpad::HTTPMultipartBuilder builder;
pid_t pid;
if (!minidump_uploader::MimeifyReport(*upload_report.get(), &builder,
&pid)) {
return false;
}
crashpad::WeakFileHandleFileWriter writer(fd);
if (!minidump_uploader::WriteBodyToFile(builder.GetBodyStream().get(),
&writer)) {
return false;
}
}
for (size_t index = 0; index < reports.size(); ++index) {
db->DeleteReport(reports[index].uuid);
}
db->CleanDatabase(0);
return true;
}
} // namespace
static jboolean JNI_AwDebug_DumpWithoutCrashing(
JNIEnv* env,
const JavaParamRef<jstring>& dump_path) {
// This may be called from any thread, and we might be in a state
// where it is impossible to post tasks, so we have to be prepared
// to do IO from this thread.
base::ThreadRestrictions::ScopedAllowIO allow_io;
base::File target(base::FilePath(ConvertJavaStringToUTF8(env, dump_path)),
base::File::FLAG_OPEN_TRUNCATED | base::File::FLAG_READ |
base::File::FLAG_WRITE);
if (!target.IsValid())
return false;
if (!CrashReporterEnabled()) {
static constexpr char kMessage[] = "WebView isn't initialized";
return static_cast<size_t>(target.WriteAtCurrentPos(
kMessage, strlen(kMessage))) == strlen(kMessage);
}
AwDebugCrashReporterClient client;
base::FilePath database_path;
if (!client.GetCrashDumpLocation(&database_path)) {
return false;
}
if (!base::CreateDirectory(database_path)) {
return false;
}
std::unique_ptr<crashpad::CrashReportDatabase> database =
crashpad::CrashReportDatabase::Initialize(database_path);
if (!database) {
return false;
}
if (!::crash_reporter::DumpWithoutCrashingForClient(&client)) {
return false;
}
return WriteLastReportToFd(database.get(), target.GetPlatformFile());
}
static void JNI_AwDebug_InitCrashKeysForWebViewTesting(JNIEnv* env) {
crash_keys::InitCrashKeysForWebViewTesting();
}
static void JNI_AwDebug_SetAllowedKeyForTesting(JNIEnv* env) {
static ::crash_reporter::CrashKeyString<32> crash_key("AW_ALLOWED_DEBUG_KEY");
crash_key.Set("AW_DEBUG_VALUE");
}
static void JNI_AwDebug_SetDeniedKeyForTesting(JNIEnv* env) {
static ::crash_reporter::CrashKeyString<32> crash_key("AW_DENIED_DEBUG_KEY");
crash_key.Set("AW_DEBUG_VALUE");
}
static void JNI_AwDebug_SetSupportLibraryWebkitVersionCrashKey(
JNIEnv* env,
const base::android::JavaParamRef<jstring>& version) {
......
......@@ -22,7 +22,6 @@ extern const char kWeblayerWebViewCompatMode[] =
// clang-format off
const char* const kWebViewCrashKeyAllowList[] = {
"AW_ALLOWED_DEBUG_KEY",
kAppPackageName,
kAppPackageVersionCode,
kAndroidSdkInt,
......
......@@ -4,12 +4,12 @@
package org.chromium.android_webview;
import org.chromium.base.Log;
import org.chromium.base.annotations.JNINamespace;
import org.chromium.base.annotations.NativeMethods;
import org.chromium.base.annotations.UsedByReflection;
import java.io.File;
import java.io.IOException;
/**
* Provides Android WebView debugging entrypoints.
......@@ -20,39 +20,18 @@ import java.io.IOException;
@JNINamespace("android_webview")
@UsedByReflection("")
public class AwDebug {
private static final String TAG = "AwDebug";
/**
* Dump webview state (predominantly a minidump for all threads,
* but including other information) to the file descriptor fd.
*
* It is expected that this method is found by reflection, as it
* is not currently exposed by the android framework, and thus it
* needs to be protected from the unwanted attention of ProGuard.
* Previously requested to dump WebView state as a minidump.
*
* The File argument must refer to a pre-existing file, which must
* be able to be re-opened for reading and writing via its
* canonical path. The file will be truncated upon re-opening.
* This is no longer supported as it doesn't include renderer state in
* multiprocess mode, significantly limiting its usefulness.
*/
@UsedByReflection("")
public static boolean dumpWithoutCrashing(File dumpFile) {
String dumpPath;
try {
dumpPath = dumpFile.getCanonicalPath();
} catch (IOException e) {
return false;
}
return AwDebugJni.get().dumpWithoutCrashing(dumpPath);
}
public static void initCrashKeysForTesting() {
AwDebugJni.get().initCrashKeysForWebViewTesting();
}
public static void setAllowedKeyForTesting() {
AwDebugJni.get().setAllowedKeyForTesting();
}
public static void setDeniedKeyForTesting() {
AwDebugJni.get().setDeniedKeyForTesting();
Log.e(TAG, "AwDebug.dumpWithoutCrashing is no longer supported.");
return false;
}
public static void setSupportLibraryWebkitVersionCrashKey(String version) {
......@@ -61,10 +40,6 @@ public class AwDebug {
@NativeMethods
interface Natives {
boolean dumpWithoutCrashing(String dumpPath);
void initCrashKeysForWebViewTesting();
void setAllowedKeyForTesting();
void setDeniedKeyForTesting();
void setSupportLibraryWebkitVersionCrashKey(String version);
}
}
// Copyright 2012 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.
package org.chromium.android_webview.test;
import static org.junit.Assert.assertNotEquals;
import static org.chromium.android_webview.test.OnlyRunIn.ProcessMode.SINGLE_PROCESS;
import android.support.test.filters.SmallTest;
import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.chromium.android_webview.AwDebug;
import org.chromium.base.test.util.DisabledTest;
import org.chromium.base.test.util.Feature;
import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.util.Scanner;
/**
* A test suite for AwDebug class.
*/
@RunWith(AwJUnit4ClassRunner.class)
@OnlyRunIn(SINGLE_PROCESS) // Only works in single-process mode, http://crbug.com/568825.
public class AwDebugTest {
@Rule
public AwActivityTestRule mActivityTestRule = new AwActivityTestRule();
private static final String TAG = "AwDebugTest";
// These constants must match android_webview/browser/aw_debug.cc.
private static final String ALLOWED_DEBUG_KEY = "AW_ALLOWED_DEBUG_KEY";
private static final String DENIED_DEBUG_KEY = "AW_DENIED_DEBUG_KEY";
private static final String DEBUG_VALUE = "AW_DEBUG_VALUE";
@Test
@SmallTest
@Feature({"AndroidWebView", "Debug"})
@DisabledTest(message = "crbug.com/913515")
public void testDump() throws Throwable {
File f = File.createTempFile("dump", ".dmp");
try {
Assert.assertTrue(AwDebug.dumpWithoutCrashing(f));
Assert.assertTrue(f.canRead());
assertNotEquals(f.length(), 0);
} finally {
Assert.assertTrue(f.delete());
}
}
@Test
@SmallTest
@Feature({"AndroidWebView", "Debug"})
@DisabledTest(message = "crbug.com/913515")
public void testDumpContainsAlloweddKey() throws Throwable {
File f = File.createTempFile("dump", ".dmp");
try {
AwDebug.initCrashKeysForTesting();
AwDebug.setAllowedKeyForTesting();
Assert.assertTrue(AwDebug.dumpWithoutCrashing(f));
assertContainsCrashKeyValue(f, ALLOWED_DEBUG_KEY, DEBUG_VALUE);
} finally {
Assert.assertTrue(f.delete());
}
}
@Test
@SmallTest
@Feature({"AndroidWebView", "Debug"})
@DisabledTest(message = "crbug.com/913515")
public void testDumpDoesNotContainDeniedKey() throws Throwable {
File f = File.createTempFile("dump", ".dmp");
try {
AwDebug.initCrashKeysForTesting();
AwDebug.setDeniedKeyForTesting();
Assert.assertTrue(AwDebug.dumpWithoutCrashing(f));
assertNotContainsCrashKeyValue(f, DENIED_DEBUG_KEY);
} finally {
Assert.assertTrue(f.delete());
}
}
private void assertNotContainsCrashKeyValue(File dump, String key) throws IOException {
String dumpContents = readEntireFile(dump);
// We are expecting the following to NOT be in the dumpContents:
//
// Content-Disposition: form-data; name="AW_DEBUG_KEY"
// AW_DEBUG_VALUE
Assert.assertFalse(dumpContents.contains(getDebugKeyLine(key)));
}
private void assertContainsCrashKeyValue(File dump, String key, String expectedValue)
throws IOException {
String dumpContents = readEntireFile(dump);
// We are expecting the following lines:
//
// Content-Disposition: form-data; name="AW_DEBUG_KEY"
// AW_DEBUG_VALUE
String debugKeyLine = getDebugKeyLine(key);
Assert.assertTrue(dumpContents.contains(debugKeyLine));
int debugKeyIndex = dumpContents.indexOf(debugKeyLine);
// Read the word after the line containing the debug key
Scanner debugValueScanner =
new Scanner(dumpContents.substring(debugKeyIndex + debugKeyLine.length()));
Assert.assertTrue(debugValueScanner.hasNext());
Assert.assertEquals(expectedValue, debugValueScanner.next());
}
private static String getDebugKeyLine(String debugKey) {
return "Content-Disposition: form-data; name=\"" + debugKey + "\"";
}
private static String readEntireFile(File file) throws IOException {
FileInputStream fileInputStream = new FileInputStream(file);
try {
byte[] data = new byte[(int) file.length()];
fileInputStream.read(data);
return new String(data);
} finally {
fileInputStream.close();
}
}
}
......@@ -235,7 +235,6 @@ instrumentation_test_apk("webview_instrumentation_test_apk") {
"../javatests/src/org/chromium/android_webview/test/AwContentsRenderTest.java",
"../javatests/src/org/chromium/android_webview/test/AwContentsStaticsTest.java",
"../javatests/src/org/chromium/android_webview/test/AwContentsTest.java",
"../javatests/src/org/chromium/android_webview/test/AwDebugTest.java",
"../javatests/src/org/chromium/android_webview/test/AwFormDatabaseTest.java",
"../javatests/src/org/chromium/android_webview/test/AwImeTest.java",
"../javatests/src/org/chromium/android_webview/test/AwJavaBridgeTest.java",
......
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