[omnibox] Stop the OmniboxPopupContentsView::result_view_at crashes

To support the secondary buttons focusing, unfocusing, and triggering
via keyboard, the OmniboxViewViews calls into OmniboxResultView via
OmniboxPopupContentsView.

In retrospect, this was a mistake, to have one View invasively start
poking into the internal state of another. We should have had both Views
interact only with OmniboxPopupModel, which is where orinj's refactor
is going.

This is leading to some Windows-only crashes, I'm guessing because
Windows has some platform-specific window manager logic that's
unexpectedly closing the popup - although I am not sure.

This is a somewhat speculative fix. Were it not for COVID, I would be
diving into the minidumps using Visual Studio and I would have a more
informed opinion of what's going on.

The ultimate solution though, is orinj's OmniboxPopupModel refactor.

Bug: 1063071
Change-Id: I33c916b86c3a7e0fbf2d3edff5732eeb4936124c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2122907
Commit-Queue: Tommy Li <tommycli@chromium.org>
Reviewed-by: manuk hovanesian <manukh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753815}

chrome/browser/ui/views/omnibox/omnibox_popup_contents_view.cc

@@ -199,6 +199,14 @@ OmniboxResultView* OmniboxPopupContentsView::result_view_at(size_t i) {
   DCHECK(!base::FeatureList::IsEnabled(omnibox::kWebUIOmniboxPopup))
       << "With the WebUI omnibox popup enabled, the code should not try to "
          "fetch the child result view.";
+
+  // TODO(tommycli): https://crbug.com/1063071
+  // Making this method public was a mistake. Outside callers have no idea about
+  // our internal state, and there's now a crash in this area. For now, let's
+  // return nullptr, but the ultimate fix is orinj's OmniboxPopupModel refactor.
+  if (i >= children().size())
+    return nullptr;
+
   return static_cast<OmniboxResultView*>(children()[i]);
 }
 

chrome/browser/ui/views/omnibox/omnibox_view_views.cc

@@ -744,6 +744,12 @@ views::Button* OmniboxViewViews::GetSecondaryButtonForSelectedLine() const {
   if (selected_line == OmniboxPopupModel::kNoMatch)
     return nullptr;
 
+  // TODO(tommycli): https://crbug.com/1063071
+  // Diving into |popup_view_| was a mistake. Here's a hotfix to stop the crash,
+  // but the ultimate fix should be to move this logic into OmniboxPopupModel.
+  if (!popup_view_ || popup_view_->result_view_at(selected_line) == nullptr)
+    return nullptr;
+
   return popup_view_->result_view_at(selected_line)->GetSecondaryButton();
 }
 
@@ -792,6 +798,12 @@ bool OmniboxViewViews::MaybeTriggerSecondaryButton(const ui::KeyEvent& event) {
   if (selected_line == OmniboxPopupModel::kNoMatch)
     return false;
 
+  // TODO(tommycli): https://crbug.com/1063071
+  // Diving into |popup_view_| was a mistake. Here's a hotfix to stop the crash,
+  // but the ultimate fix should be to move this logic into OmniboxPopupModel.
+  if (!popup_view_ || popup_view_->result_view_at(selected_line) == nullptr)
+    return false;
+
   return popup_view_->result_view_at(selected_line)
       ->MaybeTriggerSecondaryButton(event);
 }