New issue
Advanced search Search tips

Issue 649386 link

Starred by 2 users

Issue metadata

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



Sign in to add a comment

[Offline Pages] Add whitelists (or blacklists) or Prerenderer status codes that are non-retryable failures and for canceled operations

Project Member Reported by dougarnett@chromium.org, Sep 22 2016

Issue description

For background loading retry policy it would be good to differentiate between prerender failures:
  1. for canceled operations (due to some transient system condition), the attempt should not be counted as a full attempt (eg, lost network, recently visited)
  2. for hard failures with the page, we should not perform any retries (eg, creating audio stream, safe browsing issue)
 
Labels: -Type-Bug -Pri-3 Pri-2 Type-Feature
Labels: -Pri-2 Pri-3
(triage)
* Needs deign doc and/or metrics to support the lists
* Potentially obsoleted by new background-load mode
Cc: dougarnett@chromium.org dewittj@chromium.org
Here is some UMA from DEV for our prerenderer use that compels me to at least better handle RECENTLY_VISITED and CANCELLED cases from the prerender stack as these are both now handled as failures that are not retried - seems important for whenever Downloads launches (M55?)). I personally experience the RECENTLY_VISITED issue when trying to use the Download button. I will write up a bug for that and link to this. 

PrerenderFinalStatus	Quantity	PDF
RENDERER_CRASHED	2	00.29%
UNSUPPORTED_SCHEME	25	03.64%
RECENTLY_VISITED	225	32.80%
CANCELLED		419	61.08%
OPEN_URL		1	00.15%
CREATING_AUDIO_STREAM	4	00.58%
NEW_NAVIGATION_ENTRY	10	01.46%


Handling hard error like CREATING_AUDIO_STREAM will reduce latency for processing the next request if there is one (by the GcmNetworkScheduler time to give us a new window). Separately, it is a precursor to supporting a retry for non-hard failures (so it could wait for that if we want to gather more UMA on frequency).


Project Member

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

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

commit af0bd147487db4d88f06bb7a7f2e20e2286f3df1
Author: dougarnett <dougarnett@chromium.org>
Date: Wed Oct 05 19:45:54 2016

[Offline Pages] Adds classification of some prerender FinalStatus codes as canceled operations or as hard failures.
Initial set of codes are based on what I have experienced in manual evaluations
and what I am seeing in FinalStatus UMA for our Origin so far. We will want to
make decisions about more status codes in the future. Identifying the hard
failures as NO_RETRY cases is preparation for turning on a single retry for
other failures but does have an immediate benefit of trying to pick the next
request to process for hard failures rather than bail out of service window.
Another immediate benefit/effect is that I am seeing many FINAL_STATUS_CANCELLED
occurences in UMA, these would not be retried in the current code base but
they will be with this change applied.

BUG= 649386 

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

[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/offliner.h
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/request_coordinator_event_logger.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Assigned)
Project Member

Comment 6 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/+/af0bd147487db4d88f06bb7a7f2e20e2286f3df1

commit af0bd147487db4d88f06bb7a7f2e20e2286f3df1
Author: dougarnett <dougarnett@chromium.org>
Date: Wed Oct 05 19:45:54 2016

[Offline Pages] Adds classification of some prerender FinalStatus codes as canceled operations or as hard failures.
Initial set of codes are based on what I have experienced in manual evaluations
and what I am seeing in FinalStatus UMA for our Origin so far. We will want to
make decisions about more status codes in the future. Identifying the hard
failures as NO_RETRY cases is preparation for turning on a single retry for
other failures but does have an immediate benefit of trying to pick the next
request to process for hard failures rather than bail out of service window.
Another immediate benefit/effect is that I am seeing many FINAL_STATUS_CANCELLED
occurences in UMA, these would not be retried in the current code base but
they will be with this change applied.

BUG= 649386 

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

[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/chrome/browser/android/offline_pages/prerendering_loader.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/offliner.h
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/request_coordinator.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/request_coordinator_event_logger.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/components/offline_pages/background/request_coordinator_unittest.cc
[modify] https://crrev.com/af0bd147487db4d88f06bb7a7f2e20e2286f3df1/tools/metrics/histograms/histograms.xml

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

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

Sign in to add a comment