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

Issue 652889 link

Starred by 1 user
Status: Fixed
Owner:
petewil@chromium.org
Closed: Oct 2016
Cc:
dewittj@chromium.org
█ dim...@chromium.org
fgor...@chromium.org
petewil@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

For Download of pages, we need to wait for the page to load in foreground

Project Member Reported by dim...@chromium.org, Oct 4 2016 
If user is loading a page in foreground tab and hits Download button, we currently do a momentary check if page is loaded enough, and either captrue the snapshot right away or send a request to Background Offliner.

Rather, we need to start observing the loading page so in case the user waits a bit and page is loaded more, we would capture it from the live tab instead of going to Background Offliner. This will produce offline snapshot earlier, and avoid potential re-fetching of page resources. It also helps by not spawning a renderer for Background Offliner.
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 6 2016 

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

commit 80979723f0248e791feae4829095b05aac1eb3a1
Author: petewil <petewil@chromium.org>
Date: Thu Oct 06 18:31:12 2016

API changes to support suspending requests

We want to be able to suspend requests until the offline download
code is sure it won't be manually downloading them.  This API surface
change provides a mechanism to allow requests to be started in a
suspended state, and later transition to an available state.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2395903002
Cr-Commit-Position: refs/heads/master@{#423596}

[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/components/offline_pages/background/request_coordinator_unittest.cc
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 11 2016 

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

commit b1c4cc221dd01af09ceaf1eba05735046bd46d0b
Author: petewil <petewil@chromium.org>
Date: Tue Oct 11 02:37:42 2016

Implement MarkRequestCompleted.

When the foreground tab finishes a request, it will let us know, and we
need to send out the observer notification.  This code catches the
foreground request completing, and calls the observer with SUCCESS
after removing the request, and starting the next available request.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2409823002
Cr-Commit-Position: refs/heads/master@{#424346}

[modify] https://crrev.com/b1c4cc221dd01af09ceaf1eba05735046bd46d0b/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/b1c4cc221dd01af09ceaf1eba05735046bd46d0b/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/b1c4cc221dd01af09ceaf1eba05735046bd46d0b/components/offline_pages/background/request_coordinator_unittest.cc

Comment 5 by petewil@chromium.org, Oct 11 2016

Labels: Merge-Request-55
We'd like to merge these changes into M55.  The changelists associated with this bug are basically one complete change broken into several pieces for readability and ease of review.

Comment 6 by dimu@chromium.org, Oct 11 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Owner: petewil@chromium.org
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Project Member

Comment 7 by bugdroid1@chromium.org, Oct 11 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/b1210b1871cc430a82cde2335eaeb81159205c9e

commit b1210b1871cc430a82cde2335eaeb81159205c9e
Author: Pete Williamson <petewil@chromium.org>
Date: Fri Oct 07 19:11:54 2016
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 11 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/b1210b1871cc430a82cde2335eaeb81159205c9e

commit b1210b1871cc430a82cde2335eaeb81159205c9e
Author: Pete Williamson <petewil@chromium.org>
Date: Fri Oct 07 19:11:54 2016
Project Member

Comment 9 by bugdroid1@chromium.org, Oct 11 2016

Labels: -merge-approved-55 merge-merged-2883
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee359f2511345003cb17ba84f02ba788b9dae758

commit ee359f2511345003cb17ba84f02ba788b9dae758
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 11 23:08:46 2016

Implement disabled list

To promote using the original tab as the primary download source
for a page, we are making changes to disable a page for background
downloading until the foreground is done. This part of the larger
change implements the disabled list.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2395213002
Cr-Commit-Position: refs/heads/master@{#424226}
(cherry picked from commit f75ba66f9a4b29bc62d2716b3a2816cc69b8b271)

Review URL: https://codereview.chromium.org/2414553002 .

Cr-Commit-Position: refs/branch-heads/2883@{#48}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_picker.cc
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_picker.h
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_picker_unittest.cc
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 11 2016 

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

commit c7911c09971b920e0f81fa6925ab8d5536844524
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 11 23:14:44 2016

Set up a backup schedule request if we have deferred requests.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2400393002
Cr-Commit-Position: refs/heads/master@{#424315}
(cherry picked from commit 731c9896052c3824ec4630a1461738ecbccb16b5)

Review URL: https://codereview.chromium.org/2408193003 .

Cr-Commit-Position: refs/branch-heads/2883@{#49}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerBridge.java
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/chrome/browser/android/offline_pages/background_scheduler_bridge.cc
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/chrome/browser/android/offline_pages/background_scheduler_bridge.h
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/components/offline_pages/background/scheduler.h
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 11 2016 

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

commit b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 11 23:20:50 2016

Implement MarkRequestCompleted.

When the foreground tab finishes a request, it will let us know, and we
need to send out the observer notification.  This code catches the
foreground request completing, and calls the observer with SUCCESS
after removing the request, and starting the next available request.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2409823002
Cr-Commit-Position: refs/heads/master@{#424346}
(cherry picked from commit b1c4cc221dd01af09ceaf1eba05735046bd46d0b)

Review URL: https://codereview.chromium.org/2410873003 .

Cr-Commit-Position: refs/branch-heads/2883@{#50}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9/components/offline_pages/background/request_coordinator_unittest.cc

Comment 12 by dim...@chromium.org, Oct 12 2016

Status: Fixed (was: Untriaged)
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 13 2016 

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/clank/internal/apps/+/ef5d8ec36e949ac0e6434eec3d1fc4dc203615f9

commit ef5d8ec36e949ac0e6434eec3d1fc4dc203615f9
Author: Pete Williamson <petewil@chromium.org>
Date: Fri Oct 07 19:11:54 2016
Project Member

Comment 14 by bugdroid1@chromium.org, Oct 21 2016 

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

commit a07a502649e8e75f4d2b942db69d4c4511bad423
Author: petewil <petewil@chromium.org>
Date: Fri Oct 21 21:40:39 2016

Fixup a changelist where we forgot to call backgroundSchedule.

The previous changelist (https://codereview.chromium.org/2400393002/) created backgroundSchedule, and forwarded
the call in several places, but missed a spot, this corrects
that oversight.

BUG= 652889 

Review-Url: https://chromiumcodereview.appspot.com/2435393002
Cr-Commit-Position: refs/heads/master@{#426896}

[modify] https://crrev.com/a07a502649e8e75f4d2b942db69d4c4511bad423/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerBridge.java
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 27 2016

Labels: merge-merged-2840
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/80979723f0248e791feae4829095b05aac1eb3a1

commit 80979723f0248e791feae4829095b05aac1eb3a1
Author: petewil <petewil@chromium.org>
Date: Thu Oct 06 18:31:12 2016

API changes to support suspending requests

We want to be able to suspend requests until the offline download
code is sure it won't be manually downloading them.  This API surface
change provides a mechanism to allow requests to be started in a
suspended state, and later transition to an available state.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2395903002
Cr-Commit-Position: refs/heads/master@{#423596}

[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/chrome/browser/ui/webui/offline/offline_internals_ui_message_handler.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/80979723f0248e791feae4829095b05aac1eb3a1/components/offline_pages/background/request_coordinator_unittest.cc
Project Member

Comment 16 by bugdroid1@chromium.org, Oct 27 2016 

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

commit ee359f2511345003cb17ba84f02ba788b9dae758
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 11 23:08:46 2016

Implement disabled list

To promote using the original tab as the primary download source
for a page, we are making changes to disable a page for background
downloading until the foreground is done. This part of the larger
change implements the disabled list.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2395213002
Cr-Commit-Position: refs/heads/master@{#424226}
(cherry picked from commit f75ba66f9a4b29bc62d2716b3a2816cc69b8b271)

Review URL: https://codereview.chromium.org/2414553002 .

Cr-Commit-Position: refs/branch-heads/2883@{#48}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_picker.cc
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_picker.h
[modify] https://crrev.com/ee359f2511345003cb17ba84f02ba788b9dae758/components/offline_pages/background/request_picker_unittest.cc
Project Member

Comment 17 by bugdroid1@chromium.org, Oct 27 2016 

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

commit c7911c09971b920e0f81fa6925ab8d5536844524
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 11 23:14:44 2016

Set up a backup schedule request if we have deferred requests.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2400393002
Cr-Commit-Position: refs/heads/master@{#424315}
(cherry picked from commit 731c9896052c3824ec4630a1461738ecbccb16b5)

Review URL: https://codereview.chromium.org/2408193003 .

Cr-Commit-Position: refs/branch-heads/2883@{#49}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/chrome/android/java/src/org/chromium/chrome/browser/offlinepages/BackgroundSchedulerBridge.java
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/chrome/browser/android/offline_pages/background_scheduler_bridge.cc
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/chrome/browser/android/offline_pages/background_scheduler_bridge.h
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/c7911c09971b920e0f81fa6925ab8d5536844524/components/offline_pages/background/scheduler.h
Project Member

Comment 18 by bugdroid1@chromium.org, Oct 27 2016 

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

commit b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9
Author: Pete Williamson <petewil@chromium.org>
Date: Tue Oct 11 23:20:50 2016

Implement MarkRequestCompleted.

When the foreground tab finishes a request, it will let us know, and we
need to send out the observer notification.  This code catches the
foreground request completing, and calls the observer with SUCCESS
after removing the request, and starting the next available request.

BUG= 652889 

Review-Url: https://codereview.chromium.org/2409823002
Cr-Commit-Position: refs/heads/master@{#424346}
(cherry picked from commit b1c4cc221dd01af09ceaf1eba05735046bd46d0b)

Review URL: https://codereview.chromium.org/2410873003 .

Cr-Commit-Position: refs/branch-heads/2883@{#50}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/b6d6eaef2b28bbfa7cc0be5ab099cdaf0570f2c9/components/offline_pages/background/request_coordinator_unittest.cc

Comment 19 by dimu@google.com, Nov 4 2016

Labels: -merge-merged-2840
[Automated comment] removing mislabelled merge-merged-2840
►

Sign in to add a comment