New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 768797 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression : Same date is seen twice for download items after performing an undo action.

Reported by avsha...@etouch.net, Sep 26 2017

Issue description

Chrome version : 63.0.3223.8 (Official Build) 47de836373a20038efd14f8c3a154326f8c9ddda-refs/branch-heads/3223@{#11} 32/64 bit
OS : Windows(7,8,10), Linux(14.04 LTS), Mac(10.12.6)

Test URL : https://permission.site/

What steps will reproduce the problem?
1. Launch chrome, navigate to above test URL, click on 'Auto Download' button and click 'Allow' in permission bubble.
2. Click on 'Show all' button at download shelf to navigate to chrome://downloads page.
3. Now remove the first download entry from the list using 'X' icon.
4. Hit 'Ctrl + Z' keys to undo the delete action and observe the date for both the download entries.

Actual Result : Same date is seen twice for download items after performing undo action.

Expected Result : Instead, the date should appear only once for download items after performing undo action.

(Note : This issue is observed only when 2 download items are present on chrome://downloads page).

This is a regression issue broken in ‘M-56’ and using the per-revision bisect providing the bisect results,
Good build : 56.0.2901.0 (Revision : 427542)
Bad build : 56.0.2902.0 (Revision : 427892)

You are probably looking for a change made after 427882 (known good), but no later than 427883 (first known bad).

CHANGELOG URL: 
https://chromium.googlesource.com/chromium/src/+log/c4f551a8ea24ff91f8779034bd3c0ae96778cd54..9ce83c55f556e739b053e6be5f0034870be47c37

Suspect : https://chromium.googlesource.com/chromium/src/+/9ce83c55f556e739b053e6be5f0034870be47c37

@dbeam : Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to concern owner.

Thank you!
 
Expected_Result.mp4
721 KB View Download
Actual_Result.mp4
1.1 MB View Download

Comment 1 by dbeam@chromium.org, Sep 27 2017

Cc: dpa...@chromium.org
Status: Started (was: Assigned)
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45a1249b61ba15af97cd08c95976b1c87a68b53b

commit 45a1249b61ba15af97cd08c95976b1c87a68b53b
Author: Dan Beam <dbeam@chromium.org>
Date: Thu Sep 28 21:07:09 2017

Downloads page: fix rendering glitch when inserting first download

Originally, the code did this:

download.Manager = {
  insertItems_(index, items) {
    this.splice('items_', index, 0, items);
    this.updateHideDates_(index, index + items.length);  // calls .set()
  }
  // Same general pattern in #updateItem_() and #removeItem_().
}

Unfortunately, IronList#_itemsChanged() ends up ignoring the set() calls
as the "physical item" (i.e. node) is scheduled to be created, but isn't
yet.

So, instead, this CL silently creates the right data model (by setting
the data correctly and moving away from Polymer's mutation APIs), and
calls notifySlices() when it's all done and the data is in a completely
sane state.

Simply wrapping the .set() call with this.async() would also work, but I
don't like this because it:
1) re-renders templates (more work, potential flicker)
2) harder to cancel async() work or generally reason about its
   correctness

BUG= 768797 
R=dpapad@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If164268687b2a34abe745e8d43fdcf74ad4ca09a
Reviewed-on: https://chromium-review.googlesource.com/688814
Commit-Queue: Dan Beam (no longer on Chrome) <dbeam@chromium.org>
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505154}
[modify] https://crrev.com/45a1249b61ba15af97cd08c95976b1c87a68b53b/chrome/browser/resources/md_downloads/manager.js
[modify] https://crrev.com/45a1249b61ba15af97cd08c95976b1c87a68b53b/chrome/test/data/webui/BUILD.gn
[modify] https://crrev.com/45a1249b61ba15af97cd08c95976b1c87a68b53b/chrome/test/data/webui/md_downloads/downloads_browsertest.js
[delete] https://crrev.com/91958392e61a7859f888a40a858e99377b1bf1e9/chrome/test/data/webui/md_downloads/layout_tests.js
[add] https://crrev.com/45a1249b61ba15af97cd08c95976b1c87a68b53b/chrome/test/data/webui/md_downloads/manager_tests.js

Comment 3 by dbeam@chromium.org, Sep 28 2017

Status: Fixed (was: Started)

Sign in to add a comment