The attached file is a JSON formatted list of potential memory leaks.
Each entry contains an unique stackframe and the count/size of allocated memory not released.

Deleted: bug1.png 134 KB

	bug1.png 134 KB View Download

Deleted: bug2.png 143 KB

	bug2.png 143 KB View Download

Deleted: process_73717_malloc-leaks.json 3.3 MB

process_73717_malloc-leaks.json 3.3 MB View Download

More reports by chrome version:

  66.0.3355.0
     976f8bdeb3a37403
     a4404b2ec052477a
     9319e83cbbc841f9

  67.0.3361.0
     3774dfd07348e6b0
     d039da59523fc6b8
     6aacdfbb42864862

In the trace attached, I'm unable to find reference to OmniboxEditModel::OnAfterPossibleChange in the malloc heap details. Should I be able to find this?

It looks like the guidance at this point is: find the memory leak in OmniboxEditModel::OnAfterPossibleChange. Is there any further guidance? Based on my cursory read through, my guess is that the leak is in some function that's getting inlined into OnAfterPossibleChange, though it's certainly possible that I'm just missing something obvious.

Or does it make sense to just assign to an Omnibox expert?

Some observations:

1) Normally, it shouldn't be so hard to find the leak in the trace. I was able to find this, but it was ~20-30 frames deep, hidden behind a bunch of views code. 

2) At first, I thought this was a bug in the heap profiler or tracing UI. Wouldn't be the first time. Normally "base::MessagePumpCFRunLoopBase::Run" should be close to the top of the stack, in this case it's deeply nested in some views code.

3) But looking at the code in question...There's actually a nested message loop being run.
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/simple_message_box_views.cc?type=cs&q=simplemessageboxviews::show&sq=package:chromium&l=76

The fact that we're getting 100s of MBs of allocations within a nested message loop suggests that we're never exiting from the nested message loop. This is very problematic.

I'd say that the root problem is that:

chrome::ShowQuestionMessageBox calls into some views code that spins a nested message loop, which never exits. Over to Elly to find an OWNER, since this is likely related to mac-views migration of dialogs.

Deleted: Screen Shot 2018-03-22 at 1.05.14 PM.png 119 KB

	Screen Shot 2018-03-22 at 1.05.14 PM.png 119 KB View Download

I'll look into this.

I hadn't realized you could double click stack frames in the heap details view...

You're right, the memory leak is not collapsing to a single stackframe.
The leaking stackframes are not sharing the same base (see Erik's comment above).

The attached JSON file (process_73717_malloc-leaks.json) is a processed report of remaining allocations (grouped by stackframes) from the symbolized traces. If you open it in a JSON viewer, you can find the top candidates to investigate.


The following snippets are extracted from the JSON report:


"(anonymous namespace)::CreateAttributedString(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, NSColor*, NSTextAlignment)",
      "(anonymous namespace)::CreateClassifiedAttributedString(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, NSColor*, std::__1::vector<AutocompleteMatch::ACMatchClassification, std::__1::allocator<AutocompleteMatch::ACMatchClassification> > const&, signed char)",
      "-[OmniboxPopupCellData initWithMatch:image:answerImage:forDarkTheme:]",
      "-[OmniboxPopupTableController initWithMatchResults:tableView:popupView:answerImage:]",
      "OmniboxPopupViewMac::UpdatePopupAppearance()",
      "OmniboxPopupModel::OnResultChanged()",
      "OmniboxEditModel::OnCurrentMatchChanged()",
      "OmniboxController::OnResultChanged(bool)",
      "AutocompleteController::UpdateResult(bool, bool)",
      "AutocompleteController::Start(AutocompleteInput const&)",
      "OmniboxEditModel::StartAutocomplete(bool, bool)",
      "OmniboxEditModel::UpdateInput(bool, bool)",
      "OmniboxEditModel::OnAfterPossibleChange(OmniboxView::StateChanges const&, bool)",
      "OmniboxViewMac::OnAfterPossibleChange(bool)",
      "-[AutocompleteTextFieldEditor interpretKeyEvents:]",




      "(anonymous namespace)::CreateAttributedString(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, NSColor*, NSTextAlignment)",
      "(anonymous namespace)::CreateClassifiedAttributedString(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, NSColor*, std::__1::vector<AutocompleteMatch::ACMatchClassification, std::__1::allocator<AutocompleteMatch::ACMatchClassification> > const&, signed char)",
      "-[OmniboxPopupCellData initWithMatch:image:answerImage:forDarkTheme:]",
      "-[OmniboxPopupTableController initWithMatchResults:tableView:popupView:answerImage:]",
      "OmniboxPopupViewMac::UpdatePopupAppearance()",
      "OmniboxPopupModel::OnResultChanged()",
      "OmniboxEditModel::OnCurrentMatchChanged()",
      "OmniboxController::OnResultChanged(bool)",
      "AutocompleteController::UpdateResult(bool, bool)",
      "AutocompleteController::Start(AutocompleteInput const&)",
      "OmniboxEditModel::StartAutocomplete(bool, bool)",
      "OmniboxEditModel::UpdateInput(bool, bool)",
      "OmniboxEditModel::OnAfterPossibleChange(OmniboxView::StateChanges const&, bool)",
      "OmniboxViewMac::OnAfterPossibleChange(bool)",
      "-[AutocompleteTextFieldEditor interpretKeyEvents:]",





      "(anonymous namespace)::CreateAttributedString(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, NSColor*, NSTextAlignment)",
      "+[OmniboxPopupCell createSeparatorStringForDarkTheme:]",
      "OmniboxPopupViewMac::UpdatePopupAppearance()",
      "OmniboxPopupModel::OnResultChanged()",
      "OmniboxEditModel::OnCurrentMatchChanged()",
      "OmniboxController::OnResultChanged(bool)",
      "AutocompleteController::UpdateResult(bool, bool)",
      "AutocompleteController::Start(AutocompleteInput const&)",
      "OmniboxEditModel::StartAutocomplete(bool, bool)",
      "OmniboxEditModel::UpdateInput(bool, bool)",
      "OmniboxEditModel::OnAfterPossibleChange(OmniboxView::StateChanges const&, bool)",
      "OmniboxViewMac::OnAfterPossibleChange(bool)",
      "-[AutocompleteTextFieldEditor interpretKeyEvents:]",






      "(anonymous namespace)::CreateClassifiedAttributedString(std::__1::basic_string<unsigned short, base::string16_internals::string16_char_traits, std::__1::allocator<unsigned short> > const&, NSColor*, std::__1::vector<AutocompleteMatch::ACMatchClassification, std::__1::allocator<AutocompleteMatch::ACMatchClassification> > const&, signed char)",
      "-[OmniboxPopupCellData initWithMatch:image:answerImage:forDarkTheme:]",
      "-[OmniboxPopupTableController initWithMatchResults:tableView:popupView:answerImage:]",
      "OmniboxPopupViewMac::UpdatePopupAppearance()",
      "OmniboxPopupModel::OnResultChanged()",
      "OmniboxEditModel::OnCurrentMatchChanged()",
      "OmniboxController::OnResultChanged(bool)",
      "AutocompleteController::UpdateResult(bool, bool)",
      "AutocompleteController::Start(AutocompleteInput const&)",
      "OmniboxEditModel::StartAutocomplete(bool, bool)",
      "OmniboxEditModel::UpdateInput(bool, bool)",
      "OmniboxEditModel::OnAfterPossibleChange(OmniboxView::StateChanges const&, bool)",
      "OmniboxViewMac::OnAfterPossibleChange(bool)",
      "-[AutocompleteTextFieldEditor interpretKeyEvents:]",





      "OmniboxPopupViewMac::ImageForMatch(AutocompleteMatch const&) const",
      "-[OmniboxPopupTableController initWithMatchResults:tableView:popupView:answerImage:]",
      "OmniboxPopupViewMac::UpdatePopupAppearance()",
      "OmniboxPopupModel::OnResultChanged()",
      "OmniboxEditModel::OnCurrentMatchChanged()",
      "OmniboxController::OnResultChanged(bool)",
      "AutocompleteController::UpdateResult(bool, bool)",
      "AutocompleteController::Start(AutocompleteInput const&)",
      "OmniboxEditModel::StartAutocomplete(bool, bool)",
      "OmniboxEditModel::UpdateInput(bool, bool)",
      "OmniboxEditModel::OnAfterPossibleChange(OmniboxView::StateChanges const&, bool)",
      "OmniboxViewMac::OnAfterPossibleChange(bool)",
      "-[AutocompleteTextFieldEditor interpretKeyEvents:]",

I can't figure out how to get the nested message loop created in SimpleMessageBoxViews to not quit in a way that leaves the browser still useable. One way to cause it to not exit is to trigger a message box (I used a folder with 16 bookmarks, then Open All) then hit cmd-q, but after that, nothing works and the app doesn't quit properly, so there's no way to cause the omnibox to allocate memory. The general fact that nested message loops are painful is filed as  issue 404385  already.

#5: do you have any idea how I might go about getting the run loop to not exit while leaving the browser useable?

I can repro this issue by:

1) Right click a bookmarks folder with many bookmarks, select (Open all)
2) Ignore the dialog box. Press cmd + ` to switch to a different chrome window.
3) Use other Chrome window to type in Omnibox.

I confirmed by taking a sample of the browser process that the nested message loop is still spinning, and the browser is more or less functional.

#10: Huh, that's awkward. The nested message loop is still running because the dialog is still up. :\

Perhaps we should dismiss this dialog, and all the other SimpleMessageBox dialogs, when the window loses focus?

That seems like a reasonable solution.

I think that we don't technically need a nested message loop to implement a custom message box, but don't know how much plumbing would be needed to fix this.

[e.g. We'd want to make a list of all events that could cause loss of focus [e.g. tab switching, and map each one to either : be blocked, or cause dismissal].

ellyjones: are you able to make progress on the solution in #11?

I haven't looked yet, sorry. I'll try to get to it today or tomorrow.

The NextAction date has arrived: 2018-03-30

https://chromium-review.googlesource.com/c/chromium/src/+/987914

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/176db4eab527350f3dd8326a64f9ce296d9f82c5

commit 176db4eab527350f3dd8326a64f9ce296d9f82c5
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Fri Mar 30 20:06:13 2018

views: dismiss SimpleMessageBox when widget loses focus

SimpleMessageBox creates a nested runloop, and the following sequence can lead
to the browser running inside the context of that nested runloop:

1) Open a SimpleMessageBox in window 1 - the easiest way to do this is to
   "Open All" a folder with at least 16 bookmarks
2) Ignore the message box, switch to another browser window 2
3) Continue using the browser

This change dismisses the SimpleMessageBox when its widget loses activation,
which causes it to quit the nested runloop. A longer-term fix might be to change
users of SimpleMessageBox not to expect synchronous behavior, which would
remove the need for the nested runloop, but some of these uses are quite
early during browser startup and so are running in delicate conditions.

Bug:  824794 
Change-Id: I611d966e2f7b02b77d4426cbad85cb89ac82d8e4
Reviewed-on: https://chromium-review.googlesource.com/987914
Reviewed-by: Erik Chen <erikchen@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547253}
[modify] https://crrev.com/176db4eab527350f3dd8326a64f9ce296d9f82c5/chrome/browser/ui/views/simple_message_box_views.cc
[modify] https://crrev.com/176db4eab527350f3dd8326a64f9ce296d9f82c5/chrome/browser/ui/views/simple_message_box_views.h

Fixed? Do we have a way of telling if the leak goes away?

Thanks!

Assume for now that the issue is fixed.

We currently have a weekly email summary that includes frequencies of all known/unknown bugs. We'll reopen or file a new bug if the issue still recurs, and will include more details.