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

Issue 677189 link

Starred by 1 user
Status: Fixed
Owner:
dougarnett@chromium.org
Closed: Jan 2017
Cc:
dewittj@chromium.org
█ dim...@chromium.org
█ talo@chromium.org
dougarnett@chromium.org
petewil@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

[OfflinePages] Improve handling of prerendering disabled for background loading

Project Member Reported by dougarnett@chromium.org, Dec 27 2016 
We are seeing higher than expected OfflinerRequestStatus "Loading not started" bucket counts in M-55 Stable. 

One guess is that this is because prerendering is disabled on some devices and a PrerenderingLoader::CanPrerender() check in the PrerenderingOffliner::LoadAndSave() method is detecting this but without any further visibility.

At least we should add some UMA visibility for disabled prerendering when we are using the PrerenderingOffliner.

We should also consider making some features (web page download, async loading) conditionally available on whether prerendering is enabled. Unless we can switch to the new BackgroundLoader instead.

Comment 1 by dougarnett@chromium.org, Dec 28 2016 

It wasn't very obvious but I did find way to disable prerendering via flag.
Set the "No-State Prefetch" flag to Disabled (vs. Default). Otherwise, the
only other non-test path I've seen to disable it (for "offline" origin) is
in a call to PrerenderManager::SetMode() in
  chrome/browser/prerender/prerender_field_trial.cc

Comment 2 by dougarnett@chromium.org, Dec 28 2016

Labels: -Pri-3 Pri-1
Owner: dougarnett@chromium.org
Status: Assigned (was: Untriaged)
I'm starting to think that the PrerenderingOffliner (and Loader) should no longer pre-check PrerenderManager::IsPrerenderingPossible() as it is now more tied to a single mode related to the newer NoState Prefetch functionality. Also, within the PrerenderManager there are origin checks that handle our offline origin differently. I think we should simply hand off the requests to PrerenderManager
instead and let it deal with the origin-specific behavior.

Comment 3 by dim...@chromium.org, Jan 4 2017

Owner: petewil@chromium.org
Unless Doug is already working on this, it goes to Pete.

Comment 4 by petewil@chromium.org, Jan 4 2017

Owner: dougarnett@chromium.org
Doug already has a changelist for this, assigning to Doug.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 5 2017 

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

commit 9033511eba1ce7c9b06a4372df9030ccd177353b
Author: dougarnett <dougarnett@chromium.org>
Date: Thu Jan 05 03:46:22 2017

[OfflinePages] Improve visiblity/handling of "Loading not started" case
The IsPrerenderingPossible() no longer is applicable to the "offline"
origin prerender requests (it is now more tied to NoStatePrefetch
setting) so this stops prematurely checking it (via CanPrerender())
and instead passes request to PrerenderManager to let is decide whether
it accepts the "offline" origin request or not.

Also adds new Offliner RequestStatus code LOADING_NOT_ACCEPTED to
add clarity between the RC not trying to start the request vs. the
Offliner not accepting it.

BUG= 677189 

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

[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerender_adapter.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerender_adapter.h
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerender_adapter_unittest.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerendering_loader.h
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerendering_offliner.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/components/offline_pages/core/background/offliner.h
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/components/offline_pages/core/background/request_coordinator.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/components/offline_pages/core/background/request_coordinator_event_logger.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/components/offline_pages/core/background/request_coordinator_unittest.cc
[modify] https://crrev.com/9033511eba1ce7c9b06a4372df9030ccd177353b/tools/metrics/histograms/histograms.xml

Comment 6 by dougarnett@chromium.org, Jan 5 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment