Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2855663002:
Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. (Closed)
DescriptionFix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder.
The RequestFileSystemDialogView class had an unused member contents_view_ which is no longer needed after https://codereview.chromium.org/2816293002/diff/520001.
Removing this should fix the memory leak. Thanks to lazyboy for helping track down this leak.
BUG=717057
TBR=pkasting
Review-Url: https://codereview.chromium.org/2855663002
Cr-Commit-Position: refs/heads/master@{#468607}
Committed: https://chromium.googlesource.com/chromium/src/+/0eb55d6c752c6046493edcaf4fbcf222712211e9
Patch Set 1 #Patch Set 2 : Fix build error #Patch Set 3 : Fix compile errors #Patch Set 4 : Fix compile failures #Patch Set 5 : Mark dtor as virtual #Patch Set 6 : Fix redness #Patch Set 7 : Allocate the RequestFileSystemDialogView object in the ShowDialog method in the test. #Patch Set 8 : Fix compile errors #Patch Set 9 : Fix compile failures #Patch Set 10 : Remove the unused contents_view_ member from the RequestFileSystemDialogView class as it is no long… #
Messages
Total messages: 47 (41 generated)
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
ananta@chromium.org changed reviewers: + lazyboy@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...) linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView::ShowDialog() function instantiates the RequestFileSystemDialogView class and displays the dialog. This is fine in the normal case because the RequestFileSystemDialogView instance is deleted when the dialog is dismissed. This is not true for the test though. To fix this, the test is now a friend of the RequestFileSystemDialogView class. It keeps a pointer to the RequestFileSystemDialogView instance and calls the Cancel() method on it in the dtor of the test. This should fix the leak. BUG=717057 ========== to ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView::ShowDialog() function instantiates the RequestFileSystemDialogView class and displays the dialog. This is fine in the normal case because the RequestFileSystemDialogView instance is deleted when the dialog is dismissed. This is not true for the test though. To fix this, the test is now a friend of the RequestFileSystemDialogView class. It maintains the RequestFileSystemDialogView instance as a member which ensures that the object goes away BUG=717057 ==========
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView::ShowDialog() function instantiates the RequestFileSystemDialogView class and displays the dialog. This is fine in the normal case because the RequestFileSystemDialogView instance is deleted when the dialog is dismissed. This is not true for the test though. To fix this, the test is now a friend of the RequestFileSystemDialogView class. It maintains the RequestFileSystemDialogView instance as a member which ensures that the object goes away BUG=717057 ========== to ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView class had an unused member contents_view_ which is no longer needed after https://codereview.chromium.org/2816293002/diff/520001. Removing this should fix the memory leak. Thanks to lazyboy for helping track down this leak. BUG=717057 ==========
The CQ bit was checked by ananta@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by ananta@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
Description was changed from ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView class had an unused member contents_view_ which is no longer needed after https://codereview.chromium.org/2816293002/diff/520001. Removing this should fix the memory leak. Thanks to lazyboy for helping track down this leak. BUG=717057 ========== to ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView class had an unused member contents_view_ which is no longer needed after https://codereview.chromium.org/2816293002/diff/520001. Removing this should fix the memory leak. Thanks to lazyboy for helping track down this leak. BUG=717057 TBR=pkasting ==========
The CQ bit was checked by ananta@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 180001, "attempt_start_ts": 1493727917365490,
"parent_rev": "ae41773373496eaa692beb85a20cade679127d58", "commit_rev":
"0eb55d6c752c6046493edcaf4fbcf222712211e9"}
Message was sent while issue was closed.
Description was changed from ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView class had an unused member contents_view_ which is no longer needed after https://codereview.chromium.org/2816293002/diff/520001. Removing this should fix the memory leak. Thanks to lazyboy for helping track down this leak. BUG=717057 TBR=pkasting ========== to ========== Fix the memory leak in the RequestFileSystemDialogTest reported by the Chrome OS ASAN builder. The RequestFileSystemDialogView class had an unused member contents_view_ which is no longer needed after https://codereview.chromium.org/2816293002/diff/520001. Removing this should fix the memory leak. Thanks to lazyboy for helping track down this leak. BUG=717057 TBR=pkasting Review-Url: https://codereview.chromium.org/2855663002 Cr-Commit-Position: refs/heads/master@{#468607} Committed: https://chromium.googlesource.com/chromium/src/+/0eb55d6c752c6046493edcaf4fbc... ==========
Message was sent while issue was closed.
Committed patchset #10 (id:180001) as https://chromium.googlesource.com/chromium/src/+/0eb55d6c752c6046493edcaf4fbc... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
