Commit 665ad94e authored by Maggie Cai's avatar Maggie Cai Committed by Commit Bot

[IntentHandling] Add DumpWithoutCrashing for empty string for preferred apps.

There are couple of cases where when selecting "Stay In Chrome" and
"Remember my choice" actually stores an entry with empty app_id. However
this happens randomly and cannot be reproduced easily. Add the
DumpWithoutCrashing to get a trace to help debugging. The main purpose
is to identify where the empty string get generated, because logically
it doesn't seem to be possible.

In addition to this, some early return are also added to avoid storing
the entries.

BUG=853604

Change-Id: I320f29418b606288e056d38088f266c817f812e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2418279Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Maggie Cai <mxcai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#809171}
parent 07119779
......@@ -8,6 +8,7 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
#include "base/debug/dump_without_crashing.h"
#include "base/feature_list.h"
#include "base/location.h"
#include "base/stl_util.h"
......@@ -604,6 +605,12 @@ void AppServiceProxy::AddPreferredApp(const std::string& app_id,
void AppServiceProxy::AddPreferredApp(const std::string& app_id,
const apps::mojom::IntentPtr& intent) {
// TODO(https://crbug.com/853604): Remove this and convert to a DCHECK
// after finding out the root cause.
if (app_id.empty()) {
base::debug::DumpWithoutCrashing();
return;
}
auto intent_filter = FindBestMatchingFilter(intent);
if (!intent_filter) {
return;
......
......@@ -7,6 +7,7 @@
#include <utility>
#include "base/bind.h"
#include "base/debug/dump_without_crashing.h"
#include "base/stl_util.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
......@@ -116,8 +117,15 @@ void CommonAppsNavigationThrottle::OnIntentPickerClosed(
apps::AppServiceProxy* proxy =
apps::AppServiceProxyFactory::GetForProfile(profile);
if (should_persist)
proxy->AddPreferredApp(launch_name, url);
if (should_persist) {
// TODO(https://crbug.com/853604): Remove this and convert to a DCHECK
// after finding out the root cause.
if (launch_name.empty()) {
base::debug::DumpWithoutCrashing();
} else {
proxy->AddPreferredApp(launch_name, url);
}
}
if (should_launch_app) {
if (entry_type == PickerEntryType::kWeb) {
......
......@@ -6,6 +6,7 @@
#include <utility>
#include "base/debug/dump_without_crashing.h"
#include "base/feature_list.h"
#include "base/i18n/rtl.h"
#include "base/strings/string_piece.h"
......@@ -474,6 +475,11 @@ void IntentPickerBubbleView::RunCallbackAndCloseBubble(
bool should_persist) {
if (!intent_picker_cb_.is_null()) {
// Calling Run() will make |intent_picker_cb_| null.
// TODO(https://crbug.com/853604): Remove this and convert to a DCHECK
// after finding out the root cause.
if (should_persist && launch_name.empty()) {
base::debug::DumpWithoutCrashing();
}
std::move(intent_picker_cb_)
.Run(launch_name, entry_type, close_reason, should_persist);
}
......
......@@ -7,6 +7,7 @@
#include <utility>
#include "base/bind.h"
#include "base/debug/dump_without_crashing.h"
#include "base/files/file_util.h"
#include "base/json/json_string_value_serializer.h"
#include "base/metrics/histogram_macros.h"
......@@ -321,6 +322,13 @@ void AppServiceImpl::AddPreferredApp(apps::mojom::AppType app_type,
return;
}
// TODO(https://crbug.com/853604): Remove this and convert to a DCHECK
// after finding out the root cause.
if (app_id.empty()) {
base::debug::DumpWithoutCrashing();
return;
}
apps::mojom::ReplacedAppPreferencesPtr replaced_app_preferences =
preferred_apps_.AddPreferredApp(app_id, intent_filter);
......
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