Skip to content

Commit

Permalink
[m91] [memories] Don't assume incomplete visit on URL copy (part 2).
Browse files Browse the repository at this point in the history
When the omnibox URL is copied, it DCHECKs there's a pending visit. This
verifies the assumption that there shouldn't be a URL to copy unless
there's been a navigation.

But this isn't the case for chrome://blank, which isn't considered a
navigation but does display a URL in the omnibox.

Therefore, opening chrome://blank in a new tab and copying the URL
resulted in a DCHECK crash (or 'SEGV on unknown address' if DCHECKs are
disabled). The DCHECK and faulty code were introduced in the previous CL
crrev.com/c/2798278.

This CL addresses this case by checking if there's been a navigation.

(cherry picked from commit 2157536)

Bug: 1197759, 1171352
Change-Id: I34b96dadcd252e369165f626df802742ac97790a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2819679
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Auto-Submit: manuk hovanesian <manukh@chromium.org>
Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#871594}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2825490
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/branch-heads/4472@{#63}
Cr-Branched-From: 3d60439-refs/heads/master@{#870763}
  • Loading branch information
manukh authored and Chromium LUCI CQ committed Apr 14, 2021
1 parent 1edff80 commit 9042b94
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions chrome/browser/history_clusters/history_clusters_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,17 @@ HistoryClustersTabHelper::HistoryClustersTabHelper(
HistoryClustersTabHelper::~HistoryClustersTabHelper() = default;

void HistoryClustersTabHelper::OnOmniboxUrlCopied() {
DCHECK(!navigation_ids_.empty());
// It's possible that there have been no navigations if certain builtin pages
// were opened in a new tab (e.g. chrome://crash or chrome://invalid-page).
if (navigation_ids_.empty())
return;
auto* memories_service = GetMemoriesService();
// It's possible that the last navigation is complete if the tab crashed and a
// new navigation hasn't began.
if (memories_service->HasIncompleteVisit(navigation_ids_.back())) {
memories_service->GetIncompleteVisit(navigation_ids_.back())
.context_signals.omnibox_url_copied = true;
}
if (!memories_service->HasIncompleteVisit(navigation_ids_.back()))
return;
memories_service->GetIncompleteVisit(navigation_ids_.back())
.context_signals.omnibox_url_copied = true;
}

void HistoryClustersTabHelper::OnUpdatedHistoryForNavigation(
Expand Down

0 comments on commit 9042b94

Please sign in to comment.