New issue
Advanced search Search tips

Issue 610521 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Feature



Sign in to add a comment

Implement RequestCoordinator

Project Member Reported by petewil@chromium.org, May 9 2016

Issue description

For the Background Offline Pages scenario, build and implement the request coordinator described by the design doc here: https://drive.google.com/corp/drive/folders/0Bz0x-BV10ULuNkxVRGhNeVN2MXM

This will involve several steps:
Creating a skeleton C++ class that does nothing. (done)
Creating a base unit test. (done)
Creating a factory to build the class, and a unit test for it. (done)
Hook up the offline menu item to call the request coordinator. (done)
When the request arrives, put it on the request queue.
When the request arrives, ask the scheduler to schedule us
When the scheduler fires, send a task to the appropriate offliner.
When the offliner finishes, start another task if our time budget permits.
Choose the highest priority task that meets the current conditions.


 
Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, May 11 2016

Project Member

Comment 5 by bugdroid1@chromium.org, May 31 2016

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

commit 8ba587222e4138e227f9770df9d8237576342b3a
Author: petewil <petewil@chromium.org>
Date: Tue May 31 23:13:19 2016

Add the request picker and a unit test

The RequestPicker is responsible for choosing which request from the
request queue to process next.  It will apply policies from the policy
class to the data returned by the request queue, pick a likely
candidate, and call back to the request coordinator so it can start
offlining the candidate.  If there is no candidate, it will call back
to the request coordinator to let it know processing is done for this
cycle.

BUG= 610521 

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

[modify] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/components_tests.gyp
[modify] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages.gypi
[modify] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/BUILD.gn
[modify] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/request_coordinator_unittest.cc
[add] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/request_picker.cc
[add] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/request_picker.h
[add] https://crrev.com/8ba587222e4138e227f9770df9d8237576342b3a/components/offline_pages/background/request_picker_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 15 2016

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

commit ec96ac8f3893d0fbea151a1750ff15df5b5f8b9f
Author: petewil <petewil@chromium.org>
Date: Wed Jun 15 16:15:55 2016

Remove old requests from queue, start new requests if any, add test.

When offline page requests are completed, remove them from the queue of outstanding requests, and start the next request (if any)

This changelist also adds a test for the new code.

BUG= 610521 

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

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

Project Member

Comment 7 by bugdroid1@chromium.org, Jun 17 2016

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

commit 272edd820eb1d150bfdc2081729877364c061447
Author: dougarnett <dougarnett@chromium.org>
Date: Fri Jun 17 23:33:30 2016

Revises RequestCoordinator to not try to schedule another request when previous one fails.
This helps stop a failed request loop within single OfflinerTask run.
Also adds test to confirm failed requests are not removed from the queue (subject to some retry limit in future).

BUG= 610521 

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

[modify] https://crrev.com/272edd820eb1d150bfdc2081729877364c061447/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/272edd820eb1d150bfdc2081729877364c061447/components/offline_pages/background/request_coordinator_unittest.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Jun 22 2016

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

commit 0a866ec6eee17369a8fe3f565c55937fd3bf985b
Author: petewil <petewil@chromium.org>
Date: Wed Jun 22 23:38:37 2016

Don't start a second pre-render if one is in progress.

The prerenderer can only support one prerender attempt at a time.
If we are asked to start another, return false instead of starting one.
In normal operation, we will take a request off the queue when
prerendering finishes anyway.

BUG= 610521 

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

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

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 24 2016

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

commit ea61866e4aa197a1172e7460c27284e4237eb9ac
Author: petewil <petewil@chromium.org>
Date: Fri Jun 24 19:16:24 2016

Reorder RequestCoordinator functions.

To make it easier to understand the flow of a request through the
system, I re-ordered the functions to flow from top to bottom - in
most cases, the control of the async operation will call the next
function down in the file next.

No actual code changes were made, this changelist only moves existing
functions with no changes.

BUG= 610521 

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

[modify] https://crrev.com/ea61866e4aa197a1172e7460c27284e4237eb9ac/components/offline_pages/background/request_coordinator.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 24 2016

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

commit 44953f5748c4a48c619ac10770425c76346132c7
Author: petewil <petewil@chromium.org>
Date: Fri Jun 24 22:45:54 2016

fix const reference declaration

We should have declared the type as "const MyType&" instead of
"MyType const &".

BUG= 610521 

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

[modify] https://crrev.com/44953f5748c4a48c619ac10770425c76346132c7/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/44953f5748c4a48c619ac10770425c76346132c7/components/offline_pages/background/request_coordinator.h

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 1 2016

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

commit 63cc9eb76507384c24c0217ca16595e706998955
Author: petewil <petewil@chromium.org>
Date: Fri Jul 01 22:03:05 2016

Adds a prerenderer watchdog timer.

The prerenderer can take too long on a page, when it does, we should cancel
the pre-render in progress.  This change adds a timer and a test.
The actual timeout value will need tuning with real data on a 2G
impairment network and a list of real EM web pages.

BUG= 610521 

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

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

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 22 2016

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

commit 4c1748036810d9f4280dfcb4e53ebeec84abaadc
Author: petewil <petewil@chromium.org>
Date: Fri Jul 22 22:24:40 2016

More detailed implementation of the RequestPicker

BUG= 610521 

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

[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/device_conditions.h
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/offliner_policy.h
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_picker.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_picker.h
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_picker_unittest.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_queue_store_sql.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_queue_store_unittest.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/request_queue_unittest.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/save_page_request.cc
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/save_page_request.h
[modify] https://crrev.com/4c1748036810d9f4280dfcb4e53ebeec84abaadc/components/offline_pages/background/save_page_request_unittest.cc

Project Member

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

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

commit 3df75a1d847f616a8ed98aa4715333cbb5320b1d
Author: petewil <petewil@chromium.org>
Date: Wed Jul 27 20:01:28 2016

Cleanup old TODOs that got done or are no longer relevant.

BUG= 610521 

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

[modify] https://crrev.com/3df75a1d847f616a8ed98aa4715333cbb5320b1d/components/offline_pages/background/request_coordinator.cc

Project Member

Comment 19 by bugdroid1@chromium.org, Jul 29 2016

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

commit 8a57cd40185372b3257bf66434d9d845a04edf89
Author: petewil <petewil@chromium.org>
Date: Fri Jul 29 17:35:45 2016

Remove the concept of retrying a request.

To make our life simpler for shipping M54, set the max number of retries to 1.  We may turn it back up in a future milestone.

BUG= 610521 

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

[modify] https://crrev.com/8a57cd40185372b3257bf66434d9d845a04edf89/components/offline_pages/background/offliner_policy.h
[modify] https://crrev.com/8a57cd40185372b3257bf66434d9d845a04edf89/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/8a57cd40185372b3257bf66434d9d845a04edf89/components/offline_pages/background/request_picker.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Aug 3 2016

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

commit d01a2d6450812cc7f91a867621cc22e1b1f82727
Author: petewil <petewil@chromium.org>
Date: Wed Aug 03 20:53:23 2016

Provide API in RequestCoordinator to remove results by client ID.

Since the RequestQueue supports removing by request_id instead of
by client_id, I also added a new method to the RequestQueue to
remove by a list of client_ids.

BUG= 610521 

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

[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue.h
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_in_memory_store.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_in_memory_store.h
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_store.h
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_store_sql.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_store_sql.h
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_store_unittest.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/background/request_queue_unittest.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/offline_page_item.cc
[modify] https://crrev.com/d01a2d6450812cc7f91a867621cc22e1b1f82727/components/offline_pages/offline_page_item.h

Project Member

Comment 21 by bugdroid1@chromium.org, Aug 4 2016

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

commit 7b2bb2f29d275938d82f478128dc7154c6dc47a8
Author: petewil <petewil@chromium.org>
Date: Thu Aug 04 17:52:35 2016

API to provide status of save page reqeusts to API.

For components to drive background offlining programmatically, this
provides a new status API to check the current status of offline
page requests.

BUG= 610521 

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

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

Project Member

Comment 22 by bugdroid1@chromium.org, Aug 6 2016

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

commit ae875419ff203324cd94c335695a7f9c0081e197
Author: petewil <petewil@chromium.org>
Date: Sat Aug 06 00:25:22 2016

Remove the GetStatus() call from SavePageRequest.
We are moving to a new system of storing a status field.  This removes
code associated with the old way.

BUG= 610521 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[modify] https://crrev.com/ae875419ff203324cd94c335695a7f9c0081e197/chrome/browser/ui/webui/offline_internals_ui.cc
[modify] https://crrev.com/ae875419ff203324cd94c335695a7f9c0081e197/components/offline_pages/background/save_page_request.cc
[modify] https://crrev.com/ae875419ff203324cd94c335695a7f9c0081e197/components/offline_pages/background/save_page_request.h
[modify] https://crrev.com/ae875419ff203324cd94c335695a7f9c0081e197/components/offline_pages/background/save_page_request_unittest.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Aug 9 2016

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

commit 47cd270d10b49ebdaea567566f8044ae5a89ef9e
Author: petewil <petewil@chromium.org>
Date: Tue Aug 09 02:14:56 2016

Change database scheme - add state and start tracking

To be able to individually pause requests, we need a "PAUSED" state.
It needs to be persisted, so we need to store it in our RequestQueue,
which is the only persistent storage the BackgroundOfflining system has.

While we are changing the schema, we are also adding a new field to
keep track of how often a request has been started.  If a request
casues the pre-reneder to crash, we don't have code to detect the crash,
and we will just retry later.  To avoid getting stuck in an infinite
crashing loop, we only allow requests to start so many times.

Currently, we can't tell the difference between a crash and chromium
getting swapped out of memory, so this will have some false positives,
which is why we chose the limit to be higher than the completed limit.

We split the existing attempt count field into started_attempt_count
and completed_attempt_count.

BUG= 610521 

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

[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/offliner_policy.h
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_picker.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_picker_unittest.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_queue_store_sql.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_queue_store_unittest.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/request_queue_unittest.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/save_page_request.cc
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/save_page_request.h
[modify] https://crrev.com/47cd270d10b49ebdaea567566f8044ae5a89ef9e/components/offline_pages/background/save_page_request_unittest.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Aug 12 2016

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

commit 5f04432056986c94d2d5cf325a9addec96631547
Author: petewil <petewil@chromium.org>
Date: Fri Aug 12 03:04:28 2016

Changes to fit better with the needs of the download manager.

I thought that the client would only have clientIDs available, so I
created a new set of RemoveRequestByClientId functions.  However, it
turns out that the client won't be persisting these either, so I am
now removing them

They were added in https://codereview.chromium.org/2197573003/

Also, the "Get" functions now return full SavePageRequests instead
of only clientIds.

The intended usage pattern is for a client to make a request then
forget about it.  When the client wants to do something with a
request, it will first need to make a query using the
GetQueuedRequest API, then it will refer to specific requests by
request_id.

I also made RequestQueue::RemoveRequest into RemoveRequests, since
both the higher and lower level functions take a vector of request ids,
to better match the calling code.

BUG= 610521 

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

[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue.cc
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue.h
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_in_memory_store.cc
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_in_memory_store.h
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_store.h
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_store_sql.cc
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_store_sql.h
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_store_unittest.cc
[modify] https://crrev.com/5f04432056986c94d2d5cf325a9addec96631547/components/offline_pages/background/request_queue_unittest.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Aug 12 2016

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

commit 48d69bf7b3937d03580c90e06767e0f909dab6be
Author: petewil <petewil@chromium.org>
Date: Fri Aug 12 19:17:18 2016

Add an API to Pause and Resume background offlining requests.

The new API, PauseRequestsbyClientId and ResumeRequestsByClientId
is defined and implemented.  Tests are included.

It is basically parallel to the existing RemoveRequestByClientId,
and you can compare them when reviewing to speed up the review
process.

It is implemented all the way down into the request queue,
including both the in-memory queue and the SQL queue.

BUG= 610521 

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

[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_picker.cc
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue.cc
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue.h
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_in_memory_store.cc
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_in_memory_store.h
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_store.h
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_store_sql.cc
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_store_sql.h
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_store_unittest.cc
[modify] https://crrev.com/48d69bf7b3937d03580c90e06767e0f909dab6be/components/offline_pages/background/request_queue_unittest.cc

Project Member

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

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

commit feb1dd8c3f56254d000fa0c46a892c930a06054e
Author: petewil <petewil@chromium.org>
Date: Mon Aug 15 23:22:18 2016

Adds an observer for the request coordinator.

The request coordinator observer follows the standard chrome
observer_list pattern, and sends out notifications to registered
observers whenever the state of a request changes.  Supported
notifications are request succeeded, request failed, request became
available, or request was paused.

BUG= 610521 

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

[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue.cc
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue.h
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_in_memory_store.cc
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_in_memory_store.h
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_store.h
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_store_sql.cc
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_store_sql.h
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_store_unittest.cc
[modify] https://crrev.com/feb1dd8c3f56254d000fa0c46a892c930a06054e/components/offline_pages/background/request_queue_unittest.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Aug 16 2016

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

commit 1c20830eec369c944377f806b4a025e19ead9475
Author: petewil <petewil@chromium.org>
Date: Tue Aug 16 20:10:20 2016

So that the RequestCoordinator can return the offline id
of the saved page, we make it the same as the request id.

So, the RequestCoordinator will generate an offline ID, and it will get passed through to the OfflinePageModel's SavePage
method (by the prerendering offliner), which will be used
when the page is saved.

BUG= 610521 

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

[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/downloads/offline_page_download_bridge.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/offline_page_bridge.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/offline_page_tab_helper_unittest.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/offline_page_utils_unittest.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/prerendering_offliner.h
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/chrome/browser/android/offline_pages/recent_tab_helper.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/offline_page_model.h
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/offline_page_model_impl.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/offline_page_model_impl.h
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/offline_page_model_impl_unittest.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/stub_offline_page_model.cc
[modify] https://crrev.com/1c20830eec369c944377f806b4a025e19ead9475/components/offline_pages/stub_offline_page_model.h

Project Member

Comment 29 by bugdroid1@chromium.org, Aug 17 2016

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

commit 75839979d7219aa27bf0bba1674e35f3a9c07106
Author: petewil <petewil@chromium.org>
Date: Wed Aug 17 00:20:50 2016

Simplify Observer callbacks

We can merge the OnSucceeded, OnFailed, and OnRemoved callbacks into a
single OnCompleted callback, with a status to differentiate the
disposition of the request.

BUG= 610521 

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

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

Project Member

Comment 30 by bugdroid1@chromium.org, Aug 23 2016

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

commit e59d91c72518cf5a820978a46fe24672a5386ea0
Author: petewil <petewil@chromium.org>
Date: Tue Aug 23 23:03:56 2016

Cancel prerendering when a request is paused or removed.

A request might be paused or removed while being pre-rendered.  If
that happens, we should cancel the request that is inflight.

As a follow up, we still need to remember that we cancelled it, and
remove any save page that might appear after cancelling it. (there
is already a TODO in the code for that)

BUG= 610521 

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

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

Project Member

Comment 31 by bugdroid1@chromium.org, Aug 25 2016

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

commit 6f2ac6116aaf237a3525c224fc1c1be16c9c0930
Author: petewil <petewil@chromium.org>
Date: Thu Aug 25 02:17:49 2016

Always reschedule after resuming.

BUG= 610521 

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

[modify] https://crrev.com/6f2ac6116aaf237a3525c224fc1c1be16c9c0930/components/offline_pages/background/request_coordinator.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Aug 26 2016

Project Member

Comment 36 by bugdroid1@chromium.org, Sep 6 2016

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

commit 4ec7530c52fae61b0468721a427c5a80afb20b0f
Author: petewil <petewil@chromium.org>
Date: Tue Sep 06 18:50:36 2016

Rename an Enum to a slightly better name.

Resolves an outstanding TODO to choose a better name for this enum.

BUG= 610521 

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

[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/background/request_notifier.h
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/background/request_picker.cc
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/background/request_picker_unittest.cc
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/downloads/download_notifying_observer.cc
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/downloads/download_notifying_observer.h
[modify] https://crrev.com/4ec7530c52fae61b0468721a427c5a80afb20b0f/components/offline_pages/downloads/download_notifying_observer_unittest.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Sep 13 2016

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

commit 2431b127ba05526cb9dcdaea612b8aacc206d954
Author: petewil <petewil@chromium.org>
Date: Tue Sep 13 00:01:57 2016

Fix post submit problems for the vector of smart pointers change.

BruceDawson noticed that we had an unused variable, and a place
where we were not using the for_each statement that we could be
using it, this fixes those problems.

BUG= 610521 , 427616 

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

[modify] https://crrev.com/2431b127ba05526cb9dcdaea612b8aacc206d954/components/offline_pages/background/request_picker.cc

Status: Fixed (was: Assigned)
Let's consider this done, open new bugs for new changes.

Sign in to add a comment