[Offline pages] Break apart PRERENDERING_FAILED |
||
Issue descriptionToday, when we get PRERENDERING_FAILED, we do not try another request. However, PRERENDERING_FAILED is a combination of causes, some are continuable, some are not. We should break this apart into continuable and non-continuable cases, and allow the next request to proceed on a continuable case. From the mail thread: ---------------------------------- The PRERENDERING_FAILED_NO_RETRY code is defined just for that request - experienced a hard-ish failure so we should not retry that specific request. But we can/should try another one. The PRERENDERING_CANCELED code is meant to indicate that maybe now is not a good time to try any more prerendering so should not try another request yet. This happens for the UNSUPPORTED_SCHEME status and if the prerenderer cancels the operation (which can be for resource pressure). So, I think the open issue is with the PRERENDERING_FAILED code. Currently we will not try another request but maybe we should. However, there is a potential concern with trying another request if the failed request is the only available request as it could loop through all the attempts while some transient condition was causing the failure. This was a more realistic concern when we could get the RECENTLY_VISITED code as it would fail very fast. There are some codes that I wonder if we should still stop processing - MEMORY_LIMIT_EXCEEDED and RENDERED_CRASHED. There is another one I am curious about how it happens and so how we should handle continuing: NEW_NAVIGATION_ENTRY. So I wonder if we should add a new enum code for MEMORY_LIMIT_EXCEEDED and RENDERER_CRASHED at least where we do not continue processing but we do for other FAILED cases. PRERENDERING_FAILED_STOP_PROCESSING?
,
Dec 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/926b579529f4f67ab2a22364ac1d9ea1eb29a475 commit 926b579529f4f67ab2a22364ac1d9ea1eb29a475 Author: dougarnett <dougarnett@chromium.org> Date: Thu Dec 08 21:41:46 2016 [OfflinePages] Rename PRERENDERING_* result terminology to LOADING_* As we prepare for making new background loader an option over the prerendering loader, we need to either generalize Offliner and BackgroundSavePageResult result terminology or create a new set of results for the new loader. This CL generalizes that terminology. BUG= 670529 Review-Url: https://codereview.chromium.org/2566453002 Cr-Commit-Position: refs/heads/master@{#437355} [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/chrome/android/javatests/src/org/chromium/chrome/browser/offlinepages/OfflinePageSavePageLaterEvaluationTest.java [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/chrome/browser/android/offline_pages/prerendering_loader.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/chrome/browser/android/offline_pages/prerendering_loader_unittest.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/chrome/browser/android/offline_pages/prerendering_offliner_unittest.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/components/offline_pages/core/background/offliner.h [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/components/offline_pages/core/background/request_coordinator.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/components/offline_pages/core/background/request_coordinator_event_logger.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/components/offline_pages/core/background/request_coordinator_unittest.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/components/offline_pages/core/background/request_notifier.h [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/components/offline_pages/core/downloads/download_notifying_observer_unittest.cc [modify] https://crrev.com/926b579529f4f67ab2a22364ac1d9ea1eb29a475/tools/metrics/histograms/histograms.xml
,
Dec 12 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Dec 5 2016