|
|
DescriptionSeparate JumpListIcons delete task and update task
The JumpListIcons delete task deletes the existing icons in JumpListIcons,
while the update task creates the new icons, writes them into JumpListIcons
and notifies the shell. It is better to separate them so we can track
the individual cost. Since a base::SingleThreadTaskRunner is used for
posting these tasks, the tasks are always running in the correct order.
This CL also adds a base::SequencedTaskRunner for JumpListIconsOld
delete tasks as they are not required to run on a single thread.
BUG=40407, 179576
Review-Url: https://codereview.chromium.org/2807883002
Cr-Commit-Position: refs/heads/master@{#463396}
Committed: https://chromium.googlesource.com/chromium/src/+/daa4c6f41ab1da23326f2ca39c863a0384587ed6
Patch Set 1 #
Total comments: 4
Patch Set 2 : Update comments and better variables. #
Messages
Total messages: 24 (19 generated)
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 ========== Separate JumpListIcons delete task and update task. Add SequencedTaskRunner for JumpListIconsOld. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete tasks and update tasks. JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks. BUG=40407, 179576 ==========
Description was changed from ========== Separate JumpListIcons delete tasks and update tasks. JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete tasks and update tasks. JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ==========
Description was changed from ========== Separate JumpListIcons delete tasks and update tasks. JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete tasks and update tasks. JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ==========
Description was changed from ========== Separate JumpListIcons delete tasks and update tasks. JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete tasks and update tasks. The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Description was changed from ========== Separate JumpListIcons delete tasks and update tasks. The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete tasks and update tasks The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ==========
Description was changed from ========== Separate JumpListIcons delete tasks and update tasks The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete task and update task The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ==========
chengx@chromium.org changed reviewers: + gab@chromium.org
I created this CL to separate the two subtasks in task A as mentioned in the email, so that I can get their individual costs for future investigation/plan. I also added a SequencedTaskRunner to handle task B which seems more correct. PTAL~
lgtm w/ nit https://codereview.chromium.org/2807883002/diff/1/chrome/browser/win/jumplist.h File chrome/browser/win/jumplist.h (right): https://codereview.chromium.org/2807883002/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.h:190: // delete JumpListIconsOld sequentially. Update comment. https://codereview.chromium.org/2807883002/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.h:191: scoped_refptr<base::SingleThreadTaskRunner> single_thread_task_runner_; Make these variable names specific now that they're single purpose.
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.
https://codereview.chromium.org/2807883002/diff/1/chrome/browser/win/jumplist.h File chrome/browser/win/jumplist.h (right): https://codereview.chromium.org/2807883002/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.h:190: // delete JumpListIconsOld sequentially. On 2017/04/10 19:33:12, gab wrote: > Update comment. Done. https://codereview.chromium.org/2807883002/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.h:191: scoped_refptr<base::SingleThreadTaskRunner> single_thread_task_runner_; On 2017/04/10 19:33:12, gab wrote: > Make these variable names specific now that they're single purpose. Done.
The CQ bit was checked by chengx@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from gab@chromium.org Link to the patchset: https://codereview.chromium.org/2807883002/#ps20001 (title: "Update comments and better variables.")
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": 1491858590021290, "parent_rev": "02efd05d378211f550761fd26e23be5f71644a08", "commit_rev": "daa4c6f41ab1da23326f2ca39c863a0384587ed6"}
Message was sent while issue was closed.
Description was changed from ========== Separate JumpListIcons delete task and update task The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 ========== to ========== Separate JumpListIcons delete task and update task The JumpListIcons delete task deletes the existing icons in JumpListIcons, while the update task creates the new icons, writes them into JumpListIcons and notifies the shell. It is better to separate them so we can track the individual cost. Since a base::SingleThreadTaskRunner is used for posting these tasks, the tasks are always running in the correct order. This CL also adds a base::SequencedTaskRunner for JumpListIconsOld delete tasks as they are not required to run on a single thread. BUG=40407, 179576 Review-Url: https://codereview.chromium.org/2807883002 Cr-Commit-Position: refs/heads/master@{#463396} Committed: https://chromium.googlesource.com/chromium/src/+/daa4c6f41ab1da23326f2ca39c86... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/daa4c6f41ab1da23326f2ca39c86... |