New issue
Advanced search Search tips

Issue 864189 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 20
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

ResourceDownloader was never deleted after download completes

Project Member Reported by qin...@chromium.org, Jul 16

Issue description

also it is not deleted on download cancellation.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jul 18

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

commit e0b12de09e77d5954ace778522cf1a41c90f13aa
Author: Min Qin <qinmin@chromium.org>
Date: Wed Jul 18 00:56:14 2018

Fix an issue that ResourceDownloader was never deleted on download completion

This CL calls the InProgressDownloadManager to delete ResourceDownloader
after response is completed.
It also fixes the issue when download is cancelled.

BUG= 864189 

Change-Id: I184d5faaace90f49874e45b4126a0a9821fdced7
Reviewed-on: https://chromium-review.googlesource.com/1138676
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575882}
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/internal/common/BUILD.gn
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/internal/common/download_response_handler.cc
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/internal/common/resource_downloader.cc
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/internal/common/resource_downloader.h
[add] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/internal/common/url_download_request_handle.cc
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/public/common/BUILD.gn
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/public/common/download_response_handler.h
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/public/common/url_download_handler.h
[add] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/components/download/public/common/url_download_request_handle.h
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/content/browser/download/url_downloader.cc
[modify] https://crrev.com/e0b12de09e77d5954ace778522cf1a41c90f13aa/content/browser/download/url_downloader.h

Project Member

Comment 2 by bugdroid1@chromium.org, Jul 18

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

commit 9faf0129433836d06b0a7113a61332b5b7090cd3
Author: Christian Dullweber <dullweber@chromium.org>
Date: Wed Jul 18 09:00:57 2018

Revert "Fix an issue that ResourceDownloader was never deleted on download completion"

This reverts commit e0b12de09e77d5954ace778522cf1a41c90f13aa.

Reason for revert: The Cl probably introduced flakiness in multiple download related tests ( https://crbug.com/864922 )

Original change's description:
> Fix an issue that ResourceDownloader was never deleted on download completion
> 
> This CL calls the InProgressDownloadManager to delete ResourceDownloader
> after response is completed.
> It also fixes the issue when download is cancelled.
> 
> BUG= 864189 
> 
> Change-Id: I184d5faaace90f49874e45b4126a0a9821fdced7
> Reviewed-on: https://chromium-review.googlesource.com/1138676
> Commit-Queue: Min Qin <qinmin@chromium.org>
> Reviewed-by: Xing Liu <xingliu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575882}

TBR=qinmin@chromium.org,shaktisahu@chromium.org,xingliu@chromium.org

Change-Id: I669a69fc135547508b44ccc8150847cc5824b2ec
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864189 
Reviewed-on: https://chromium-review.googlesource.com/1141684
Reviewed-by: Christian Dullweber <dullweber@chromium.org>
Commit-Queue: Christian Dullweber <dullweber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575981}
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/internal/common/BUILD.gn
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/internal/common/download_response_handler.cc
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/internal/common/resource_downloader.cc
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/internal/common/resource_downloader.h
[delete] https://crrev.com/da0888bbe572b42c9baf96f63fd78aa872b5a832/components/download/internal/common/url_download_request_handle.cc
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/public/common/BUILD.gn
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/public/common/download_response_handler.h
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/components/download/public/common/url_download_handler.h
[delete] https://crrev.com/da0888bbe572b42c9baf96f63fd78aa872b5a832/components/download/public/common/url_download_request_handle.h
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/content/browser/download/url_downloader.cc
[modify] https://crrev.com/9faf0129433836d06b0a7113a61332b5b7090cd3/content/browser/download/url_downloader.h

Project Member

Comment 3 by bugdroid1@chromium.org, Jul 18

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

commit 937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2
Author: Min Qin <qinmin@chromium.org>
Date: Wed Jul 18 20:35:02 2018

Reland "Fix an issue that ResourceDownloader was never deleted on download completion"

This reverts commit 9faf0129433836d06b0a7113a61332b5b7090cd3.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Revert "Fix an issue that ResourceDownloader was never deleted on download completion"
> 
> This reverts commit e0b12de09e77d5954ace778522cf1a41c90f13aa.
> 
> Reason for revert: The Cl probably introduced flakiness in multiple download related tests ( https://crbug.com/864922 )
> 
> Original change's description:
> > Fix an issue that ResourceDownloader was never deleted on download completion
> > 
> > This CL calls the InProgressDownloadManager to delete ResourceDownloader
> > after response is completed.
> > It also fixes the issue when download is cancelled.
> > 
> > BUG= 864189 
> > 
> > Change-Id: I184d5faaace90f49874e45b4126a0a9821fdced7
> > Reviewed-on: https://chromium-review.googlesource.com/1138676
> > Commit-Queue: Min Qin <qinmin@chromium.org>
> > Reviewed-by: Xing Liu <xingliu@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575882}
> 
> TBR=qinmin@chromium.org,shaktisahu@chromium.org,xingliu@chromium.org
> 
> Change-Id: I669a69fc135547508b44ccc8150847cc5824b2ec
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  864189 
> Reviewed-on: https://chromium-review.googlesource.com/1141684
> Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> Commit-Queue: Christian Dullweber <dullweber@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#575981}

TBR=qinmin@chromium.org,shaktisahu@chromium.org,dullweber@chromium.org,xingliu@chromium.org

Change-Id: I11012d0bf159f4944d69cc13881180db9ed98d7e
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864189 
Reviewed-on: https://chromium-review.googlesource.com/1142171
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576190}
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/internal/common/BUILD.gn
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/internal/common/download_response_handler.cc
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/internal/common/resource_downloader.cc
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/internal/common/resource_downloader.h
[add] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/internal/common/url_download_request_handle.cc
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/public/common/BUILD.gn
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/public/common/download_response_handler.h
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/public/common/url_download_handler.h
[add] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/components/download/public/common/url_download_request_handle.h
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/content/browser/download/url_downloader.cc
[modify] https://crrev.com/937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2/content/browser/download/url_downloader.h

Project Member

Comment 4 by bugdroid1@chromium.org, Jul 18

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

commit 164631f61a5819f1644adb80131e2a653d6916ec
Author: Min Qin <qinmin@chromium.org>
Date: Wed Jul 18 20:39:20 2018

Revert "Reland "Fix an issue that ResourceDownloader was never deleted on download completion""

This reverts commit 937dccc2caf6b73d9fcff24225bb8ff66a0a5ec2.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> Reland "Fix an issue that ResourceDownloader was never deleted on download completion"
> 
> This reverts commit 9faf0129433836d06b0a7113a61332b5b7090cd3.
> 
> Reason for revert: <INSERT REASONING HERE>
> 
> Original change's description:
> > Revert "Fix an issue that ResourceDownloader was never deleted on download completion"
> > 
> > This reverts commit e0b12de09e77d5954ace778522cf1a41c90f13aa.
> > 
> > Reason for revert: The Cl probably introduced flakiness in multiple download related tests ( https://crbug.com/864922 )
> > 
> > Original change's description:
> > > Fix an issue that ResourceDownloader was never deleted on download completion
> > > 
> > > This CL calls the InProgressDownloadManager to delete ResourceDownloader
> > > after response is completed.
> > > It also fixes the issue when download is cancelled.
> > > 
> > > BUG= 864189 
> > > 
> > > Change-Id: I184d5faaace90f49874e45b4126a0a9821fdced7
> > > Reviewed-on: https://chromium-review.googlesource.com/1138676
> > > Commit-Queue: Min Qin <qinmin@chromium.org>
> > > Reviewed-by: Xing Liu <xingliu@chromium.org>
> > > Cr-Commit-Position: refs/heads/master@{#575882}
> > 
> > TBR=qinmin@chromium.org,shaktisahu@chromium.org,xingliu@chromium.org
> > 
> > Change-Id: I669a69fc135547508b44ccc8150847cc5824b2ec
> > No-Presubmit: true
> > No-Tree-Checks: true
> > No-Try: true
> > Bug:  864189 
> > Reviewed-on: https://chromium-review.googlesource.com/1141684
> > Reviewed-by: Christian Dullweber <dullweber@chromium.org>
> > Commit-Queue: Christian Dullweber <dullweber@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#575981}
> 
> TBR=qinmin@chromium.org,shaktisahu@chromium.org,dullweber@chromium.org,xingliu@chromium.org
> 
> Change-Id: I11012d0bf159f4944d69cc13881180db9ed98d7e
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug:  864189 
> Reviewed-on: https://chromium-review.googlesource.com/1142171
> Reviewed-by: Min Qin <qinmin@chromium.org>
> Commit-Queue: Min Qin <qinmin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#576190}

TBR=qinmin@chromium.org,shaktisahu@chromium.org,dullweber@chromium.org,xingliu@chromium.org

Change-Id: Ia2f90aa688078db7191cf9eeb59608619194a256
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  864189 
Reviewed-on: https://chromium-review.googlesource.com/1142305
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576195}
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/internal/common/BUILD.gn
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/internal/common/download_response_handler.cc
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/internal/common/resource_downloader.cc
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/internal/common/resource_downloader.h
[delete] https://crrev.com/7431adcf2c10bc966992660ceed9324cf4d9705a/components/download/internal/common/url_download_request_handle.cc
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/public/common/BUILD.gn
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/public/common/download_response_handler.h
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/components/download/public/common/url_download_handler.h
[delete] https://crrev.com/7431adcf2c10bc966992660ceed9324cf4d9705a/components/download/public/common/url_download_request_handle.h
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/content/browser/download/url_downloader.cc
[modify] https://crrev.com/164631f61a5819f1644adb80131e2a653d6916ec/content/browser/download/url_downloader.h

Project Member

Comment 5 by bugdroid1@chromium.org, Jul 19

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

commit f45ea5c26ad6f63ead6a555909d7b4129c656a0b
Author: Min Qin <qinmin@chromium.org>
Date: Thu Jul 19 07:05:13 2018

Reland: Fix an issue that ResourceDownloader was never deleted on download completion

This CL calls the InProgressDownloadManager to delete ResourceDownloader
after response is completed.
It also fixes the issue when download is cancelled.
The new CL fixes an issue that StreamHandleInputStream::OnStreamCompleted() could get called twice

TBR=xingliu@chromium.org
BUG= 864189 

Change-Id: Ia46040ceee88fd9978911c26ee32765a1ac4a099
Reviewed-on: https://chromium-review.googlesource.com/1142462
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576401}
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/internal/common/BUILD.gn
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/internal/common/download_response_handler.cc
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/internal/common/resource_downloader.cc
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/internal/common/resource_downloader.h
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/internal/common/stream_handle_input_stream.cc
[add] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/internal/common/url_download_request_handle.cc
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/public/common/BUILD.gn
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/public/common/download_response_handler.h
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/public/common/url_download_handler.h
[add] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/components/download/public/common/url_download_request_handle.h
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/content/browser/download/url_downloader.cc
[modify] https://crrev.com/f45ea5c26ad6f63ead6a555909d7b4129c656a0b/content/browser/download/url_downloader.h

Status: Fixed (was: Assigned)

Sign in to add a comment