Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2852763003:
Delete JumpList temp file when failing to write the icon's content to it (Closed)
|
Created:
3 years, 7 months ago by chengx Modified:
3 years, 7 months ago Reviewers:
grt (UTC plus 2) CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionDelete JumpList temp file when failing to write the icon's content to it
Currently, an empty temp file is created before a JumpList icon's
content is written to it. However, the temp file is not deleted right
away if the content writing fails, which explains why there're empty
icon temp files in the jumplist folder sometimes. This indicates we're
leaking temp files and may cause performance issues.
This CL fixes this issue by deleting the empty temp files right away
when necessary.
BUG=40407, 179576, 715902, 716601
Review-Url: https://codereview.chromium.org/2852763003
Cr-Commit-Position: refs/heads/master@{#468357}
Committed: https://chromium.googlesource.com/chromium/src/+/ced3697c2cfa3e1d5814f8bbf9456aaf4a75543b
Patch Set 1 #
Total comments: 2
Patch Set 2 : Fix comments. #Messages
Total messages: 33 (28 generated)
Description was changed from ========== Delete the empty temp file to avoid leakage BUG=40407, 179576 ========== to ========== Delete the temp file right away if fails to write the icon's content to it Currently, an empty temp file is created before the icon's content is written to it. However, it's not deleted right away if it fails to write the content. This can lead to icons' leakage. This CL fixes this issue by delete the empty temp file right away when necessary. BUG=40407, 179576 ==========
Description was changed from ========== Delete the temp file right away if fails to write the icon's content to it Currently, an empty temp file is created before the icon's content is written to it. However, it's not deleted right away if it fails to write the content. This can lead to icons' leakage. This CL fixes this issue by delete the empty temp file right away when necessary. BUG=40407, 179576 ========== to ========== Delete the JumpList icon temp file if fails to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by delete the empty temp files right away when necessary. BUG=40407, 179576 ==========
Description was changed from ========== Delete the JumpList icon temp file if fails to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by delete the empty temp files right away when necessary. BUG=40407, 179576 ========== to ========== Delete the JumpList icon temp file if fails to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576 ==========
Description was changed from ========== Delete the JumpList icon temp file if fails to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576 ========== to ========== Delete JumpList temp file if fails to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576 ==========
Description was changed from ========== Delete JumpList temp file if fails to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576 ==========
Description was changed from ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902 ==========
Description was changed from ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content, which explains there're empty icon temp files in the jumplist folder sometimes. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902 ==========
The CQ bit was checked by chengx@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 ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content, which explains there're empty icon temp files in the jumplist folder sometimes. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content, which explains there're empty icon temp files in the jumplist folder sometimes. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 ==========
Description was changed from ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before the JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content, which explains there're empty icon temp files in the jumplist folder sometimes. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before a JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content, which explains there're empty icon temp files in the jumplist folder sometimes. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 ==========
Description was changed from ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before a JumpList icon's content is written to it. However, the temp file is not deleted right away if it fails to write the content, which explains there're empty icon temp files in the jumplist folder sometimes. This can lead to temp files' leakage. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before a JumpList icon's content is written to it. However, the temp file is not deleted right away if the content writing fails, which explains why there're empty icon temp files in the jumplist folder sometimes. This indicates we're leaking temp files and may cause performance issues. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 ==========
chengx@chromium.org changed reviewers: + grt@chromium.org
Hi Greg, thanks for catching this bug. PTAL~
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
Patchset #1 (id:1) has been deleted
The CQ bit was checked by chengx@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 with a nit https://codereview.chromium.org/2852763003/diff/20001/chrome/browser/win/jump... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2852763003/diff/20001/chrome/browser/win/jump... chrome/browser/win/jumplist.cc:120: base::DeleteFile(path, false); nit: put the comment above DeleteFile rather than above the condition. maybe simplify to: // Delete the file created by CreateTemporaryFileInDir since it will not be used.
The CQ bit was checked by chengx@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...
I've fixed the nit in the new patch set. Thanks! https://codereview.chromium.org/2852763003/diff/20001/chrome/browser/win/jump... File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2852763003/diff/20001/chrome/browser/win/jump... chrome/browser/win/jumplist.cc:120: base::DeleteFile(path, false); On 2017/05/01 08:47:32, grt (UTC plus 2) wrote: > nit: put the comment above DeleteFile rather than above the condition. maybe > simplify to: > // Delete the file created by CreateTemporaryFileInDir since it will not be > used. Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by chengx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from grt@chromium.org Link to the patchset: https://codereview.chromium.org/2852763003/#ps40001 (title: "Fix comments.")
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": 40001, "attempt_start_ts": 1493662052046220,
"parent_rev": "039ec1ad2eaec6f13bfdda8cb3ac5e5d8cdc32b3", "commit_rev":
"ced3697c2cfa3e1d5814f8bbf9456aaf4a75543b"}
Message was sent while issue was closed.
Description was changed from ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before a JumpList icon's content is written to it. However, the temp file is not deleted right away if the content writing fails, which explains why there're empty icon temp files in the jumplist folder sometimes. This indicates we're leaking temp files and may cause performance issues. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 ========== to ========== Delete JumpList temp file when failing to write the icon's content to it Currently, an empty temp file is created before a JumpList icon's content is written to it. However, the temp file is not deleted right away if the content writing fails, which explains why there're empty icon temp files in the jumplist folder sometimes. This indicates we're leaking temp files and may cause performance issues. This CL fixes this issue by deleting the empty temp files right away when necessary. BUG=40407, 179576, 715902, 716601 Review-Url: https://codereview.chromium.org/2852763003 Cr-Commit-Position: refs/heads/master@{#468357} Committed: https://chromium.googlesource.com/chromium/src/+/ced3697c2cfa3e1d5814f8bbf945... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/chromium/src/+/ced3697c2cfa3e1d5814f8bbf945... |
