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

Issue 709639 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit 22 days ago
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Prerenderer is downloading an error page

Project Member Reported by chili@chromium.org, Apr 7 2017

Issue description

Chrome version: ToT build

Repro steps:

1. Make sure background loader flag is off
2. Turn ON airplane mode
3. Navigate to a page that doesn't exist (i.e. www.cnm.com)
4. Queue another page (i.e. www.google.com)
5. press home button (THIS STEP IS IMPORTANT)
6. Turn on airplane mode (DO NOT TRY TO OPEN CHROME)
7. Wait as the first request downloads. It will succeed
8. Click on the notification to see the error page: INTERNET_DISCONNECTED
 

Comment 1 by jianli@chromium.org, Apr 11 2017

Note for repro steps:
Queuing another page is not needed. Just press "DOWNLOAD PAGE LATER" for the page that doesn't exist.

Comment 2 by jianli@chromium.org, Apr 11 2017

Cc: dewittj@chromium.org
It seems that we simply do nothing in PrerenderContents::DidFinishNavigation when navigation_handle->IsErrorPage().

This might be related to patch https://codereview.chromium.org/2653023002. The author made the following comment regarding error page:
  IsErrorPage() is added to maintain the same behavior of DidFinishNavigation as
DidNavigateMainFrame (since the latter doesn't fire it for errors).

  For PrerenderContents, in case of an error, we actually don't get an
IsErrorPage() to true unless in some races. The timing is that there are two
DidStartNavigation/DidFinishNavigation sets of callbacks. The first one is for
the load that doesn't find the page. It's not the error page yet so
IsErrorPage() is false. The second pair is for the loading of the error page, so
IsErrorPage() is true. The issue is that the renderer sends the prefetch
complete message that you added recently between these two pairs, so usually
with the thread hopping/loading times the second pair is not seen.

Based on my debugging, DidStartNavigation/DidFinishNavigation is only triggered once from PrerenderContents. In DidFinishNavigation, IsErrorPage() is true.


Project Member

Comment 3 by bugdroid1@chromium.org, Apr 12 2017

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

commit 2d54317a97881484713af4069f01677e924dc87e
Author: chili <chili@chromium.org>
Date: Wed Apr 12 18:10:55 2017

[Offline pages] Add new Error page failure status inside the MHTML archiver and update corresponding histogram.

This should record number of error pages we are seeing being recorded by Prerender and also eliminate this altogether.

BUG= 709639 ,692688

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

[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/chrome/browser/android/offline_pages/offline_page_mhtml_archiver.cc
[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h
[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/chrome/browser/android/offline_pages/offline_page_mhtml_archiver_unittest.cc
[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/components/offline_pages/core/offline_page_archiver.h
[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/components/offline_pages/core/offline_page_model_impl.cc
[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/components/offline_pages/core/offline_page_types.h
[modify] https://crrev.com/2d54317a97881484713af4069f01677e924dc87e/tools/metrics/histograms/histograms.xml

Owner: chili@chromium.org
Status: Fixed (was: Assigned)
Chili fixed this, assigning it to her so she gets credit for fixing it, and marking it fixed.

Sign in to add a comment