New issue
Advanced search Search tips

Issue 686121 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 2
Type: Bug



Sign in to add a comment

Set offline delay to 1.5 second

Project Member Reported by olivierrobin@chromium.org, Jan 27 2017

Issue description

The delay that triggers offline page is too short and triggers too quickly.

Increase it from 1000, 2000, 3000 ms to reach 25, 50, 75% page load to 
1500, 3000, 4500ms.
 
Labels: M-57
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 27 2017

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

commit 6f74a028c85e2224bb450ef4f340dc20250cdab8
Author: olivierrobin <olivierrobin@chromium.org>
Date: Fri Jan 27 17:07:21 2017

Set Offline trigger delay to 1500ms.

Delay is too short to allow pages to load before offline page is displayed.
Increase it to 1500ms, 3000ms, 4500ms to reach 25%, 50%, 75% of the load.

BUG= 686121 

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

[modify] https://crrev.com/6f74a028c85e2224bb450ef4f340dc20250cdab8/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm

Status: Fixed (was: Started)
Mardini: is the experience better?
Should we cherry-pick to M-57?
Yes, please. We might need to add more delay though in the future. Let's see if we hear any feedback about this.
Labels: Merge-Request-57
Project Member

Comment 7 by sheriffbot@chromium.org, Feb 2 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 8 by bugdroid1@chromium.org, Feb 2 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/56bd58dff5e0e7c9e921b7a0ec91c549d49eafa1

commit 56bd58dff5e0e7c9e921b7a0ec91c549d49eafa1
Author: Olivier Robin <olivierrobin@chromium.org>
Date: Thu Feb 02 10:41:46 2017

Set Offline trigger delay to 1500ms.

Delay is too short to allow pages to load before offline page is displayed.
Increase it to 1500ms, 3000ms, 4500ms to reach 25%, 50%, 75% of the load.

BUG= 686121 

Review-Url: https://codereview.chromium.org/2663473003
Cr-Commit-Position: refs/heads/master@{#446697}
(cherry picked from commit 6f74a028c85e2224bb450ef4f340dc20250cdab8)

Review-Url: https://codereview.chromium.org/2668383003 .
Cr-Commit-Position: refs/branch-heads/2987@{#269}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/56bd58dff5e0e7c9e921b7a0ec91c549d49eafa1/ios/chrome/browser/reading_list/reading_list_web_state_observer.mm

Status: Verified (was: Fixed)
Checked on the chrome beta version 57.0.2987.35 on iPhone 7 plus with iOS 10.2.1.  Added reading entry.  
Switched off wifi.
Opened offline reading list item.

Checked the delay in opening the offline page. Looks good.
mardini@: did you see any improvement after this fix?


Yes. The number of overtriggering cases is much lower now. Thank you.

Sign in to add a comment