Commit 186a65c8 authored by Joe DeBlasio's avatar Joe DeBlasio Committed by Commit Bot

[Mixed Content DL Blocking] Exclude known cases from dump-without-crashing.

This CL updates the cases in which mixed content download blocking will
dump without crashing by excluding now-known cases where initiators are
missing. These include downloads from webview/cct, internally-initiated
downloads and user-initiated downloads of offline pages.

It also silences dumps from a not-yet-understood situation where
seemingly-normal downloads don't have initiators. The stack trace isn't
helping in this case, so crash dumps aren't useful.

Bug: 1029003
Change-Id: I77102e361d334e847331f5e0bc1b9ce5321ab4a8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940723
Commit-Queue: Joe DeBlasio <jdeblasio@chromium.org>
Auto-Submit: Joe DeBlasio <jdeblasio@chromium.org>
Reviewed-by: default avatarMustafa Emre Acer <meacer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719731}
parent 702cbe72
...@@ -22,6 +22,8 @@ ...@@ -22,6 +22,8 @@
#include "url/gurl.h" #include "url/gurl.h"
#include "url/origin.h" #include "url/origin.h"
using download::DownloadSource;
namespace { namespace {
// Map the string file extension to the corresponding histogram enum. // Map the string file extension to the corresponding histogram enum.
...@@ -183,16 +185,38 @@ bool ShouldBlockFileAsMixedContent(const base::FilePath& path, ...@@ -183,16 +185,38 @@ bool ShouldBlockFileAsMixedContent(const base::FilePath& path,
is_inferred = true; is_inferred = true;
} }
if (is_inferred && auto download_source = item.GetDownloadSource();
item.GetDownloadSource() != download::DownloadSource::CONTEXT_MENU) { auto transition_type = item.GetTransitionType();
// Report a trace if we inferred an initiator in an unexpected way. We expect
// this to happen with:
// - context-menu-initiated downloads,
// - user-initiated downloads of offline pages on Android,
// - requests in e.g., webview/CCT.
//
// TODO(1029082): We also occasionally find 'regular' navigations without
// initiators (NAVIGATION/PAGE_TRANSITION_LINK). The trace doesn't help in
// this case, so ignore them here.
if (is_inferred && download_source != DownloadSource::CONTEXT_MENU &&
download_source != DownloadSource::OFFLINE_PAGE &&
!(transition_type & ui::PAGE_TRANSITION_FROM_API) &&
!(download_source == DownloadSource::NAVIGATION &&
PageTransitionTypeIncludingQualifiersIs(transition_type,
ui::PAGE_TRANSITION_LINK))) {
ReportTrace(base::StringPrintf("inferred initiator [%d, %x]", ReportTrace(base::StringPrintf("inferred initiator [%d, %x]",
item.GetDownloadSource(), download_source, transition_type));
item.GetTransitionType()));
} }
if (!initiator.has_value()) {
// Report a trace if we still don't have an initiator. This mostly happens
// with INTERNAL_API calls, which are OK.
//
// TODO(1029062): INTERNAL_API is also used for background fetch. That
// probably isn't the correct behavior, since INTERNAL_API is otherwise used
// for Chrome stuff. Background fetch should probably be HTTPS-only.
if (!initiator.has_value() &&
download_source != DownloadSource::INTERNAL_API) {
ReportTrace(base::StringPrintf("no initiator found [%d, %x]", ReportTrace(base::StringPrintf("no initiator found [%d, %x]",
item.GetDownloadSource(), download_source, transition_type));
item.GetTransitionType()));
} }
// Then see if that extension is blocked // Then see if that extension is blocked
......
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