Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(142)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 18051015: Use scoped_ptr for ShellWindowLinkDelegate to prevent memory leak. (Closed)

Created:
7 years, 5 months ago by zhchbin
Modified:
7 years, 5 months ago
Reviewers:
sky
CC:
chromium-reviews, benwells
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Use scoped_ptr for ShellWindowLinkDelegate to prevent memory leak. BUG=NONE Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210761

Patch Set 1 #

Total comments: 4

Patch Set 2 : Add virtual destructor. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+22 lines, -14 lines) Patch
M chrome/browser/ui/apps/chrome_shell_window_delegate.h View 1 3 chunks +16 lines, -0 lines 0 comments Download
M chrome/browser/ui/apps/chrome_shell_window_delegate.cc View 1 3 chunks +6 lines, -14 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
zhchbin
Please review.
7 years, 5 months ago (2013-07-07 13:49:45 UTC) #1
sky
https://codereview.chromium.org/18051015/diff/1/chrome/browser/ui/apps/chrome_shell_window_delegate.cc File chrome/browser/ui/apps/chrome_shell_window_delegate.cc (right): https://codereview.chromium.org/18051015/diff/1/chrome/browser/ui/apps/chrome_shell_window_delegate.cc#newcode85 chrome/browser/ui/apps/chrome_shell_window_delegate.cc:85: new_contents->SetDelegate(shell_window_link_delegate_.get()); What is the lifetime of new_contents? https://codereview.chromium.org/18051015/diff/1/chrome/browser/ui/apps/chrome_shell_window_delegate.h File ...
7 years, 5 months ago (2013-07-08 14:51:39 UTC) #2
zhchbin
https://codereview.chromium.org/18051015/diff/1/chrome/browser/ui/apps/chrome_shell_window_delegate.cc File chrome/browser/ui/apps/chrome_shell_window_delegate.cc (right): https://codereview.chromium.org/18051015/diff/1/chrome/browser/ui/apps/chrome_shell_window_delegate.cc#newcode85 chrome/browser/ui/apps/chrome_shell_window_delegate.cc:85: new_contents->SetDelegate(shell_window_link_delegate_.get()); On 2013/07/08 14:51:39, sky wrote: > What is ...
7 years, 5 months ago (2013-07-09 02:00:10 UTC) #3
sky
LGTM
7 years, 5 months ago (2013-07-09 16:03:53 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zhchbin@gmail.com/18051015/6001
7 years, 5 months ago (2013-07-10 01:04:48 UTC) #5
commit-bot: I haz the power
Step "update" is always a major failure. Look at the try server FAQ for more ...
7 years, 5 months ago (2013-07-10 02:44:01 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/zhchbin@gmail.com/18051015/6001
7 years, 5 months ago (2013-07-10 02:50:13 UTC) #7
commit-bot: I haz the power
7 years, 5 months ago (2013-07-10 06:31:18 UTC) #8
Message was sent while issue was closed. 
Change committed as 210761

Powered by Google App Engine
This is Rietveld 408576698