Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2844463003:
Fix the number of JumpList slots assigned to each JumpList category (Closed)
DescriptionFix the number of JumpList slots assigned to each JumpList category
By default, the maximum number of items to display in JumpList is 10
per Windows. Previously, we considered there'are 10 available JumpList
slots, and we assign these slots to "Most visit" and "Recently closed"
categories accordingly. However, each category title takes one slot.
Therefore, the number of JumpList slots assigned to each category is
incorrect. This caused bug 714925, "Recently closed category of
JumpList won't show up until closing three websites instead of one".
To fix this, we need to adjust the number of available JumpList slots
based on the presence of the two JumpList categories. If one of the
categories presents, there'll be one available slot less. If both
categories present, there'll be two available slots less.
Since now the number of available slots is 1 or 2 less as it should
be, we're now deleting and creating 1 or 2 less icons per JumpList
update. This saves some disk IO. Note that the current implementation
deletes 10 icons followed by creating 10 icons per JumpList update.
BUG=714925, 40407, 179576
Review-Url: https://codereview.chromium.org/2844463003
Cr-Commit-Position: refs/heads/master@{#467094}
Committed: https://chromium.googlesource.com/chromium/src/+/b60799e04934ed8a06d47beafffd1d28b995915f
Patch Set 1 #
Total comments: 4
Patch Set 2 : Address comments #Messages
Total messages: 23 (18 generated)
Description was changed from ========== Fix available jumplist slots count BUG= ========== to ========== Fix the number of jumplist slots assigned to each jumplist category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available jumplist slots, and we assign these slots to each category accordingly. However, each category title, e.g., "Most visited" takes one slot, therefore the number of jumplist slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available jumplist slots based on the presence of both jumplist categories. If one of the categories presents, there'll one available slot less. If both categories present, there'll two available slots less. Since now the available slots are 1 or 2 less as it should be, we're now deleting and creating 1 or 2 icons per jumplist update now. This saves some disk IO. BUG=714925, 40407, 179576 ==========
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 ========== Fix the number of jumplist slots assigned to each jumplist category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available jumplist slots, and we assign these slots to each category accordingly. However, each category title, e.g., "Most visited" takes one slot, therefore the number of jumplist slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available jumplist slots based on the presence of both jumplist categories. If one of the categories presents, there'll one available slot less. If both categories present, there'll two available slots less. Since now the available slots are 1 or 2 less as it should be, we're now deleting and creating 1 or 2 icons per jumplist update now. This saves some disk IO. BUG=714925, 40407, 179576 ========== to ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per jumplist update. This saves some disk IO. BUG=714925, 40407, 179576 ==========
chengx@chromium.org changed reviewers: + gab@chromium.org, grt@chromium.org
This change was part of crrev.com/2836873003. Since it has a different focus, now I put it in a separate CL. PTAL~
lgtm https://codereview.chromium.org/2844463003/diff/1/chrome/browser/win/jumplist.cc File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2844463003/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.cc:224: // Adjust the available jumplist slots accordingly. // Adjust the available jumplist slots to account for the category titles. https://codereview.chromium.org/2844463003/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.cc:227: user_max_items_adjusted--; pre-decrement here and below --user_max_items_adjusted;
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 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...
All the feedback comments are addressed. Thanks! https://codereview.chromium.org/2844463003/diff/1/chrome/browser/win/jumplist.cc File chrome/browser/win/jumplist.cc (right): https://codereview.chromium.org/2844463003/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.cc:224: // Adjust the available jumplist slots accordingly. On 2017/04/25 18:43:11, gab wrote: > // Adjust the available jumplist slots to account for the category titles. Done. https://codereview.chromium.org/2844463003/diff/1/chrome/browser/win/jumplist... chrome/browser/win/jumplist.cc:227: user_max_items_adjusted--; On 2017/04/25 18:43:11, gab wrote: > pre-decrement here and below > > --user_max_items_adjusted; Done.
Description was changed from ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per jumplist update. This saves some disk IO. BUG=714925, 40407, 179576 ========== to ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation updates 10 icons per update. BUG=714925, 40407, 179576 ==========
Description was changed from ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation updates 10 icons per update. BUG=714925, 40407, 179576 ========== to ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation updates 10 icons per JumpList update. BUG=714925, 40407, 179576 ==========
Description was changed from ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation updates 10 icons per JumpList update. BUG=714925, 40407, 179576 ========== to ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation deletes 10 icons followed by creating 10 icons per JumpList update. BUG=714925, 40407, 179576 ==========
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 gab@chromium.org Link to the patchset: https://codereview.chromium.org/2844463003/#ps20001 (title: "Address 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": 20001, "attempt_start_ts": 1493151392624230,
"parent_rev": "08d35f601c7cfbdc66f9a7f8836e32028d344559", "commit_rev":
"b60799e04934ed8a06d47beafffd1d28b995915f"}
Message was sent while issue was closed.
Description was changed from ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation deletes 10 icons followed by creating 10 icons per JumpList update. BUG=714925, 40407, 179576 ========== to ========== Fix the number of JumpList slots assigned to each JumpList category By default, the maximum number of items to display in JumpList is 10 per Windows. Previously, we considered there'are 10 available JumpList slots, and we assign these slots to "Most visit" and "Recently closed" categories accordingly. However, each category title takes one slot. Therefore, the number of JumpList slots assigned to each category is incorrect. This caused bug 714925, "Recently closed category of JumpList won't show up until closing three websites instead of one". To fix this, we need to adjust the number of available JumpList slots based on the presence of the two JumpList categories. If one of the categories presents, there'll be one available slot less. If both categories present, there'll be two available slots less. Since now the number of available slots is 1 or 2 less as it should be, we're now deleting and creating 1 or 2 less icons per JumpList update. This saves some disk IO. Note that the current implementation deletes 10 icons followed by creating 10 icons per JumpList update. BUG=714925, 40407, 179576 Review-Url: https://codereview.chromium.org/2844463003 Cr-Commit-Position: refs/heads/master@{#467094} Committed: https://chromium.googlesource.com/chromium/src/+/b60799e04934ed8a06d47beafffd... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/b60799e04934ed8a06d47beafffd... |
