Issue metadata
Sign in to add a comment
|
[YouTube] A PAUSED download of offline copy of page remains PAUSED and a new copy of offline page starts downloading automatically when closing the tab and opening a NTP
Reported by
ahalder@etouch.net,
Dec 20 2016
|
||||||||||||||||||||||
Issue descriptionApplication Version: 56.0.2924.34 Android Build Number: 7.1.1/NMF26O Device: Google Pixel Steps to reproduce: 1. Launch chrome > Go to m.youtube.com 2. PLAY any video 3. Save the page offline and pause the download when in progress 4. Close the tab and open a NTP 5. Observe Observed behavior: The PAUSED file remains PAUSED and a new copy of offline page starts downloading automatically Expected behavior: THE PAUSED file remain PAUSED or RESUME download but no new copy of offline page should start downloading automatically Frequency: <5/5> Additional comments: Last good Build: 56.0.2924.3 First Bad Build: 56.0.2924.4 This issue is reproducible on Android devices ex. Google Pixel (7.1.1/NMF26O), Google Pixel XL (7.1/NDE63V), Spice Mi-498 (6.0.1/MOB30W), Samsung Galaxy S4 (5.0.1/LRX22C), Karbonn Sparkle V (5.1.1/LMY47V), Samsung Galaxy J2 (5.1.1/LMY47X), Samsung Galaxy J7 (6.0.1/MMB29K), Nexus 7 (6.0.1/MOB30X) and Nexus 9 (7.0/NRD91N) Bisect Range: https://chromium.googlesource.com/chromium/src/+log/56.0.2924.3..56.0.2924.4?pretty=fuller&n=10000
,
Dec 20 2016
Suspected CL: https://chromium.googlesource.com/chromium/src/+/d7cde35af699b567f1cd88ffbebd3b12623f133f Review-Url: https://codereview.chromium.org/2520583002 @fgorski, Can you please look in to this, Thanks!
,
Dec 20 2016
Code in suspected CL has nothing to do with the problem. Assigning to Pete and looping in Doug and Dmitry. Also, I don't think this blocks stable, but leave it to Dmitry to decide.
,
Dec 20 2016
This bug is in RequestCoordinator::EnableForOffliner() where we are marking the request as AVAILABLE (via MarkAttemptAborted()) without checking if it was actually in PAUSED state instead of OFFLINING state. Perhaps we would make the MarkAttemptAbortedTask conditional on request being found in OFFLINING state or else we might need another Task.
,
Dec 21 2016
I don't think I would block stable for this, but we'll request a M56 merge anyway, the fix would be nice to have.
,
Dec 21 2016
,
Dec 21 2016
,
Dec 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff7cfc31cc358e47ed3b311a97fb6f9a728507dc commit ff7cfc31cc358e47ed3b311a97fb6f9a728507dc Author: petewil <petewil@chromium.org> Date: Wed Dec 21 21:13:57 2016 Fix for crbug.com/675863 - don't clobber PAUSE state. When making a page available for offlining, we were clobbering the pause state. We now check to only change the state to AVAILABLE if it was previously OFFLINING, allowing the PAUSE state to remain unchanged. BUG= 675863 Review-Url: https://codereview.chromium.org/2598573003 Cr-Commit-Position: refs/heads/master@{#440212} [modify] https://crrev.com/ff7cfc31cc358e47ed3b311a97fb6f9a728507dc/components/offline_pages/core/background/mark_attempt_aborted_task_unittest.cc [modify] https://crrev.com/ff7cfc31cc358e47ed3b311a97fb6f9a728507dc/components/offline_pages/core/background/save_page_request.cc [modify] https://crrev.com/ff7cfc31cc358e47ed3b311a97fb6f9a728507dc/components/offline_pages/core/background/save_page_request.h
,
Dec 21 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Dec 22 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dddbcafe16533b7f638a8a0fa8155267b3cbb52 commit 2dddbcafe16533b7f638a8a0fa8155267b3cbb52 Author: Pete Williamson <petewil@chromium.org> Date: Thu Dec 22 00:47:21 2016 Fix for crbug.com/675863 - don't clobber PAUSE state. When making a page available for offlining, we were clobbering the pause state. We now check to only change the state to AVAILABLE if it was previously OFFLINING, allowing the PAUSE state to remain unchanged. Original change this was cherry picked from: Review-Url: https://codereview.chromium.org/2598573003 Cr-Commit-Position: refs/heads/master@{#440212} (manually cherry picked from commit ff7cfc31cc358e47ed3b311a97fb6f9a728507dc) BUG= 675863 R=dewittj@chromium.org Review-Url: https://codereview.chromium.org/2599593002 . Cr-Commit-Position: refs/branch-heads/2924@{#594} Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059} [modify] https://crrev.com/2dddbcafe16533b7f638a8a0fa8155267b3cbb52/components/offline_pages/background/mark_attempt_aborted_task_unittest.cc [modify] https://crrev.com/2dddbcafe16533b7f638a8a0fa8155267b3cbb52/components/offline_pages/background/save_page_request.cc [modify] https://crrev.com/2dddbcafe16533b7f638a8a0fa8155267b3cbb52/components/offline_pages/background/save_page_request.h
,
Jan 2 2017
This issue is now fixed on latest M56-56.0.2924.47 and M57-57.0.2969.0 Thanks!
,
Jan 2 2017
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by ahalder@etouch.net
, Dec 20 2016