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

Issue 682766 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Having more LOADING_CANCELED on 2g-poor with Android N

Project Member Reported by romax@chromium.org, Jan 19 2017

Issue description

After I upgraded my Nexus 6 from a Android L, I run the test harness on GIN-2g-poor and I'm getting more LOADING_CANCELED as request_status returned in OfflinerDoneCallback than before. Digging more I found most of them are due to 'prerender::FINAL_STATUS_UNSUPPORTED_SCHEME'. I guess it happens when we have no network (even if we're connected to wifi)?

I did a pseudo-'side-by-side' run last night:
Setting A: Nexus 6 with Android N on GIN-2g-poor, with changes of snapshotting on last retry on top of TOT.
Setting B: Nexus 6 with Android L on GIN-2g-poor, TOT build.
Both were using the 100-url list. And they started within 5 minutes (i only have one codebase checked out so i have to build another one and run, will see if we can do it based on built APK).
Results:
Setting A: Total requested URLs: 100, Completed: 100, Failed: 65, Failure Rate: 65.00% (Mostly START_COUNT_EXCEEDED)
Setting B: Total requested URLs: 100, Completed: 100, Failed: 29, Failure Rate: 29.00% ({START/RETRY}_COUNT_EXCEEDED mixed 6/26 respectively)

We might need to find the root cause and make some tweaks to prerenderer status handling if needed.
 
This bug only shows intermittently.  Current theory is that we are trying requests when chrome is offline (which is 10-20 seconds out of every minute on 2G poor), and they are getting prerenderer error 22, which indicates that the device is offline.

Potential long term fix is to check conditions from android instead of chrome, since chrome does not get network change notifications while in background mode. 
This may also be related to the fact that the test scheduler does not check the network conditions before issuing a request, and it bypasses the normal GCM NM network checks.
Project Member

Comment 3 by bugdroid1@chromium.org, Jan 25 2017

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

commit 76d8b4dc116f6e4e2c59b4d0b79fb82dacd4f6a9
Author: romax <romax@chromium.org>
Date: Wed Jan 25 03:00:37 2017

[Offline Pages] Adding a network check in test scheduler.

The test scheduler wasn't behaving as GCMNetworkManager since it had no
idea if the network is gone at the moment it was called back from request
coodinator. Adding a check which would prevent the test scheduler from
calling startImmediateProcessing when there's no network.

BUG= 682766 

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

[modify] https://crrev.com/76d8b4dc116f6e4e2c59b4d0b79fb82dacd4f6a9/chrome/browser/android/offline_pages/evaluation/evaluation_test_scheduler.cc

Status: Started (was: Assigned)
This will be fixed by getting the net state from android instead of NetworkChangeNotifier.

Comment 5 by romax@chromium.org, Jan 27 2017

On the other hand, I think this might be the issue where the test scheduler wasn't checking network conditions when calling RequestCoordinator::StartImmediateProcessing(). It would call RC to start processing even if there's no network condition, which is not the case in real scenarios. 
Also, since RC would callback to the scheduler (test scheduler in test harness case) when connection is lost, while the test scheduler didn't have any mechanism to deal with the case, it would call start processing immediately.


This issue has been fixed by posting start processing call if connection is missing while the test scheduler is asked to scheduler a request. And there's a improvement on how the results look like.

But I'm still not against improving the connection detection ability in our code.
Status: Fixed (was: Started)
This should be fixed now that we treat error 22 as retryable.

Sign in to add a comment