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

Issue 601173 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Create and integrate background offlining interface to PrerenderManager

Project Member Reported by dougarnett@chromium.org, Apr 6 2016

Issue description

Depends on new PrerenderManager support (https://bugs.chromium.org/p/chromium/issues/detail?id=599500)


 
Design references:
  http://go/chrome-background-loading  # concerning intergration to PrerenderManager
  http://go/chrome-background-offlining  # top-level design document
Project Member

Comment 3 by bugdroid1@chromium.org, May 10 2016

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

commit 30fe6bc61105ba5c555c735166b362e4986485b5
Author: dougarnett <dougarnett@chromium.org>
Date: Tue May 10 18:14:06 2016

Updates to PrerenderingOffliner and PrerenderingLoader interfaces to push
PrerenderManager knowledge down into Loader only. Also adds initial
unit tests for Offliner with mocked Loader. This CL to be followed by
a Loader CL that integrates to PrerenderManager. Fleshing out the Offliner
logic more will come after that.

BUG= 601173 

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

[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/browser/android/offline_pages/prerendering_loader.h
[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/browser/android/offline_pages/prerendering_offliner.h
[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/browser/android/offline_pages/prerendering_offliner_factory.cc
[add] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/30fe6bc61105ba5c555c735166b362e4986485b5/components/offline_pages/background/offliner.h

Project Member

Comment 4 by bugdroid1@chromium.org, May 24 2016

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

commit 8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce
Author: dougarnett <dougarnett@chromium.org>
Date: Tue May 24 00:39:20 2016

PrerenderingLoader initial integration with PrerenderManager/PrerenderHandle and make it unit-testable.

It proved challenging to write unittests wrt mocking PrerenderManager and/or PrerenderHandler and be able to create a stub PrerenderHandler without touching the prerender stack code so this CL takes the approach of adding an adapter between the Loader and prerender stack calls so that they can be intercepted for testing.

BTW, PrerenderingLoader is actually the "BackgroundLoader" of go/chrome-background-loading

BUG= 601173 

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

[add] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerender_adapter.cc
[add] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerender_adapter.h
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerendering_loader.h
[add] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerendering_offliner.h
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/chrome_browser.gypi
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/components/offline_pages/background/offliner.h
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce/components/offline_pages/background/request_coordinator_unittest.cc

Project Member

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

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

commit 9c983d12e207d07c7cb0922eccc102e6c7475f31
Author: mvanouwerkerk <mvanouwerkerk@chromium.org>
Date: Wed May 25 13:49:55 2016

Revert of PrerenderingLoader initial integration with PrerenderManager/PrerenderHandle (patchset #22 id:420001 of https://codereview.chromium.org/1968593002/ )

Reason for revert:
The unit tests are failing on asan-clang-phone, I'm sorry.

https://bugs.chromium.org/p/chromium/issues/detail?id=614699

Original issue's description:
> PrerenderingLoader initial integration with PrerenderManager/PrerenderHandle and make it unit-testable.
>
> It proved challenging to write unittests wrt mocking PrerenderManager and/or PrerenderHandler and be able to create a stub PrerenderHandler without touching the prerender stack code so this CL takes the approach of adding an adapter between the Loader and prerender stack calls so that they can be intercepted for testing.
>
> BTW, PrerenderingLoader is actually the "BackgroundLoader" of go/chrome-background-loading
>
> BUG= 601173 
>
> Committed: https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce
> Cr-Commit-Position: refs/heads/master@{#395485}

TBR=fgorski@chromium.org,pasko@chromium.org,petewil@chromium.org,gabadie@chromium.org,dougarnett@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 601173 

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

[delete] https://crrev.com/bdc444ee2eebb0970f239cc6f651c4e794915c37/chrome/browser/android/offline_pages/prerender_adapter.cc
[delete] https://crrev.com/bdc444ee2eebb0970f239cc6f651c4e794915c37/chrome/browser/android/offline_pages/prerender_adapter.h
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/browser/android/offline_pages/prerendering_loader.h
[delete] https://crrev.com/bdc444ee2eebb0970f239cc6f651c4e794915c37/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/browser/android/offline_pages/prerendering_offliner.h
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/chrome_browser.gypi
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/components/offline_pages/background/offliner.h
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/9c983d12e207d07c7cb0922eccc102e6c7475f31/components/offline_pages/background/request_coordinator_unittest.cc

Project Member

Comment 6 by bugdroid1@chromium.org, May 26 2016

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

commit bd2f237c4acbdd9e0f6495b688f2c3da86127cac
Author: dougarnett <dougarnett@chromium.org>
Date: Thu May 26 16:00:14 2016

PrerenderingLoader initial integration with PrerenderManager/PrerenderHandle and make it unit-testable.

[Note: this CL landed and was reverted per test failure reported in  issue 614699 .]

It proved challenging to write unittests wrt mocking PrerenderManager and/or PrerenderHandler and be able to create a stub PrerenderHandler without touching the prerender stack code so this CL takes the approach of adding an adapter between the Loader and prerender stack calls so that they can be intercepted for testing.

BTW, PrerenderingLoader is actually the "BackgroundLoader" of go/chrome-background-loading

TBR=fgorski@chromium.org,petewil@chromium.org,pasko@chromium.org,gabadie@chromium.org
BUG= 601173 , 614699 

Committed: https://crrev.com/8ed74fd7c401a1ff9a5a57c270409e0f66f6d3ce
Cr-Commit-Position: refs/heads/master@{#395485}

patch from issue 1968593002 at patchset 420001 (http://crrev.com/1968593002#ps420001)

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

[add] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerender_adapter.cc
[add] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerender_adapter.h
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerendering_loader.h
[add] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerendering_offliner.h
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/chrome_browser.gypi
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/chrome/chrome_tests_unit.gypi
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/components/offline_pages/background/offliner.h
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/components/offline_pages/background/request_coordinator.h
[modify] https://crrev.com/bd2f237c4acbdd9e0f6495b688f2c3da86127cac/components/offline_pages/background/request_coordinator_unittest.cc

Project Member

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

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

commit d83899ec45e736c2a9289c1fe0b61572e31a112d
Author: dougarnett <dougarnett@chromium.org>
Date: Thu Jun 09 20:18:07 2016

Integrates the SnapshotController into the PrerenderingLoader to provide logic for determining when the right moment is to consider the page loaded and ready for snapshotting.
BUG= 601173 

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

[modify] https://crrev.com/d83899ec45e736c2a9289c1fe0b61572e31a112d/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/d83899ec45e736c2a9289c1fe0b61572e31a112d/chrome/browser/android/offline_pages/prerendering_loader.h
[modify] https://crrev.com/d83899ec45e736c2a9289c1fe0b61572e31a112d/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc

Status: Fixed (was: Assigned)
I have exercised this integration successfully so closing now.

Project Member

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

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

commit d83899ec45e736c2a9289c1fe0b61572e31a112d
Author: dougarnett <dougarnett@chromium.org>
Date: Thu Jun 09 20:18:07 2016

Integrates the SnapshotController into the PrerenderingLoader to provide logic for determining when the right moment is to consider the page loaded and ready for snapshotting.
BUG= 601173 

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

[modify] https://crrev.com/d83899ec45e736c2a9289c1fe0b61572e31a112d/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/d83899ec45e736c2a9289c1fe0b61572e31a112d/chrome/browser/android/offline_pages/prerendering_loader.h
[modify] https://crrev.com/d83899ec45e736c2a9289c1fe0b61572e31a112d/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc

Sign in to add a comment