Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2913173002:
Remove font cache deletion code (Closed)
DescriptionRemove font cache deletion code
This code was intended to be removed in 2017 and was causing
ProfileHelperTest.OpenNewWindowForProfile browser test to be flaky
on my local machine.
BUG=728144
Review-Url: https://codereview.chromium.org/2913173002
Cr-Commit-Position: refs/heads/master@{#475951}
Committed: https://chromium.googlesource.com/chromium/src/+/366c8bf9d9543605d97f63865b38b898876f2d22
Patch Set 1 #
Total comments: 6
Patch Set 2 : Delete cleanup code #Messages
Total messages: 21 (10 generated)
ftirelo@chromium.org changed reviewers: + gab@chromium.org, thakis@chromium.org
+scottmg who I believe worked on directwrite a while ago https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_main_win.cc (right): https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... chrome/browser/chrome_browser_main_win.cc:356: // TODO(kulshin): remove this cleanup code in 2017. http://crbug.com/603718 Given that it's 2017 now, can we just delete this whole call?
https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_main_win.cc (left): https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... chrome/browser/chrome_browser_main_win.cc:362: content::BrowserThread::FILE), Surprised that using the FILE thread would have made this flaky. https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_main_win.cc (right): https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... chrome/browser/chrome_browser_main_win.cc:356: // TODO(kulshin): remove this cleanup code in 2017. http://crbug.com/603718 On 2017/05/31 14:44:12, Nico wrote: > Given that it's 2017 now, can we just delete this whole call? Yes we can, this was cleanup added for a year in April 2016.
Description was changed from ========== Post font cache deletion to the profile task runner. This was causing ProfileHelperTest.OpenNewWindowForProfile browser test to be flaky on my local machine. BUG=728144 ========== to ========== Remove font cache deletion code This code was intended to be removed in 2017 and was causing ProfileHelperTest.OpenNewWindowForProfile browser test to be flaky on my local machine. BUG=728144 ==========
Deleted the code and updated the CL description. Will dry-run on the CQ to check if any test needs to be updated. https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_main_win.cc (left): https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... chrome/browser/chrome_browser_main_win.cc:362: content::BrowserThread::FILE), On 2017/05/31 14:55:01, gab wrote: > Surprised that using the FILE thread would have made this flaky. Acknowledged. https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... File chrome/browser/chrome_browser_main_win.cc (right): https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... chrome/browser/chrome_browser_main_win.cc:356: // TODO(kulshin): remove this cleanup code in 2017. http://crbug.com/603718 On 2017/05/31 14:55:01, gab wrote: > On 2017/05/31 14:44:12, Nico wrote: > > Given that it's 2017 now, can we just delete this whole call? > > Yes we can, this was cleanup added for a year in April 2016. Done. https://codereview.chromium.org/2913173002/diff/1/chrome/browser/chrome_brows... chrome/browser/chrome_browser_main_win.cc:356: // TODO(kulshin): remove this cleanup code in 2017. http://crbug.com/603718 On 2017/05/31 14:44:12, Nico (vacation Jun 3-11) wrote: > Given that it's 2017 now, can we just delete this whole call? Done.
The CQ bit was checked by ftirelo@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...
lgtm
lgtm
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 ftirelo@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": 20001, "attempt_start_ts": 1496250237235490,
"parent_rev": "bd8aaee01b0dcf66a19f52ec97d63700a8b4c66a", "commit_rev":
"366c8bf9d9543605d97f63865b38b898876f2d22"}
Message was sent while issue was closed.
Description was changed from ========== Remove font cache deletion code This code was intended to be removed in 2017 and was causing ProfileHelperTest.OpenNewWindowForProfile browser test to be flaky on my local machine. BUG=728144 ========== to ========== Remove font cache deletion code This code was intended to be removed in 2017 and was causing ProfileHelperTest.OpenNewWindowForProfile browser test to be flaky on my local machine. BUG=728144 Review-Url: https://codereview.chromium.org/2913173002 Cr-Commit-Position: refs/heads/master@{#475951} Committed: https://chromium.googlesource.com/chromium/src/+/366c8bf9d9543605d97f63865b38... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/366c8bf9d9543605d97f63865b38...
Message was sent while issue was closed.
scottmg@chromium.org changed reviewers: + scottmg@chromium.org
Message was sent while issue was closed.
lgtm please comment on and close crbug.com/603718 too.
Message was sent while issue was closed.
On 2017/05/31 17:19:36, scottmg wrote: > lgtm please comment on and close crbug.com/603718 too. Closed. Thanks for the reminder.
Message was sent while issue was closed.
On 2017/05/31 17:19:36, scottmg wrote: > lgtm please comment on and close crbug.com/603718 too. Closed. Thanks for the reminder. |
