New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 636632 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 1
Type: Bug



Sign in to add a comment

Fix many memory leaks in chrome/browser/ui/cocoa/omnibox.

Project Member Reported by erikc...@chromium.org, Aug 11 2016

Issue description

Each time I type or delete a character from the omnibox, UpdatePopupApperance is
called at least 3 times. Each call to OmniboxPopupViewMac::UpdatePopupApperance
leaks an NSViewController*, as well as all omnibox results.

This should be merged to M-53, once the fix has landed in Canary and is stable.
 
Cc: rnimmagadda@chromium.org
@erikchen: Friendly Ping! We are nearing M53 stable release, we need to get this issue fixed before the same.

Could you please provide an update on this issue.

I really appreciate your help.

Thank you!
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 12 2016

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

commit b87e1820a06ac5dd1d9881be5342286fac920e43
Author: erikchen <erikchen@chromium.org>
Date: Fri Aug 12 14:01:28 2016

Fix many memory leaks in chrome/browser/ui/cocoa/omnibox.

Each time I type or delete a character from the omnibox, UpdatePopupApperance is
called at least 3 times. Each call to OmniboxPopupViewMac::UpdatePopupApperance
was leaking an NSViewController*, as well as all omnibox results.

BUG= 636632 

Review-Url: https://codereview.chromium.org/2233123003
Cr-Commit-Position: refs/heads/master@{#411637}

[modify] https://crrev.com/b87e1820a06ac5dd1d9881be5342286fac920e43/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h
[modify] https://crrev.com/b87e1820a06ac5dd1d9881be5342286fac920e43/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
[modify] https://crrev.com/b87e1820a06ac5dd1d9881be5342286fac920e43/chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.h
[modify] https://crrev.com/b87e1820a06ac5dd1d9881be5342286fac920e43/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

Status: Fixed (was: Started)
Labels: Merge-Request-53

Comment 5 by dimu@chromium.org, Aug 13 2016

Labels: -Merge-Request-53 Merge-Approved-53 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M53 (branch: 2785)

Comment 6 by gov...@chromium.org, Aug 14 2016

Please merge your change by Monday (08/15) 5:00 PM PT so we can take it in for next week Beta release. Thank you.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 15 2016

Labels: -merge-approved-53 merge-merged-2785
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7e53ed2c594fff414ec56f1ae97bfbd297f1d352

commit 7e53ed2c594fff414ec56f1ae97bfbd297f1d352
Author: erikchen <erikchen@chromium.org>
Date: Mon Aug 15 22:22:22 2016

[Merge to 2785] Fix many memory leaks in chrome/browser/ui/cocoa/omnibox.

> Each time I type or delete a character from the omnibox, UpdatePopupApperance is
> called at least 3 times. Each call to OmniboxPopupViewMac::UpdatePopupApperance
> was leaking an NSViewController*, as well as all omnibox results.
>
> BUG= 636632 
>
> Review-Url: https://codereview.chromium.org/2233123003
> Cr-Commit-Position: refs/heads/master@{#411637}
> (cherry picked from commit b87e1820a06ac5dd1d9881be5342286fac920e43)

Review URL: https://codereview.chromium.org/2252523002 .

Cr-Commit-Position: refs/branch-heads/2785@{#611}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

[modify] https://crrev.com/7e53ed2c594fff414ec56f1ae97bfbd297f1d352/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.h
[modify] https://crrev.com/7e53ed2c594fff414ec56f1ae97bfbd297f1d352/chrome/browser/ui/cocoa/omnibox/omnibox_popup_cell.mm
[modify] https://crrev.com/7e53ed2c594fff414ec56f1ae97bfbd297f1d352/chrome/browser/ui/cocoa/omnibox/omnibox_popup_matrix.h
[modify] https://crrev.com/7e53ed2c594fff414ec56f1ae97bfbd297f1d352/chrome/browser/ui/cocoa/omnibox/omnibox_popup_view_mac.mm

Cc: ashej...@chromium.org
@erikchen: Hey, are there any manual steps for TE to verify here ?

If so, I request you to provide the same to verify the issue.

Appreciate your help.

Thank you!
It's likely possible to use either Instruments or chrome://tracing to verify this, but there is some complexity involved. If TEs are in general interested in the topic of looking into memory leaks, I could probably give a talk about it.

Sign in to add a comment