New issue
Advanced search Search tips

Issue 908725 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Components:
EstimatedDays: ----
NextAction: 2018-12-12
OS: Android
Pri: 2
Type: Bug

Blocked on:
issue 919566

Blocking:
issue 850624



Sign in to add a comment

Navigation Predictor: Experiment with preconnecting to main frame origin when the tab is brought to foreground

Project Member Reported by tbansal@chromium.org, Nov 27

Issue description

On Android, if a previously hidden tab is brought to foreground, it's likely that user may click an element on that webpage. e.g, this may happen when user brings Chrome to the foreground. We should experiment with preconnecting to the main frame origin when the tab is brought to the foreground. Preconnecting is expected to help with the navigation performance if the user eventually clicks on a link on the webpage.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 27

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

commit 7b6a767fe5148f5d7c3c43da360e812b376e6bb0
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Nov 27 15:01:47 2018

Navigation Predictor: Preconnect on tab foreground

When the tab is brought to the foreground, preconnect to the
origin of the main frame request. This preconnection is done
at most once, and is helpful in case user clicks on a link.

Change-Id: I6786c14ad7be0921bb309aabac5a75cff9819db8
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1350209
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611107}
[modify] https://crrev.com/7b6a767fe5148f5d7c3c43da360e812b376e6bb0/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/7b6a767fe5148f5d7c3c43da360e812b376e6bb0/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/7b6a767fe5148f5d7c3c43da360e812b376e6bb0/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[modify] https://crrev.com/7b6a767fe5148f5d7c3c43da360e812b376e6bb0/tools/metrics/histograms/enums.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 27

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

commit b195e066366cd83bd4f6abc4430ad31fede89895
Author: Ken Rockot <rockot@google.com>
Date: Tue Nov 27 17:18:39 2018

Revert "Navigation Predictor: Preconnect on tab foreground"

This reverts commit 7b6a767fe5148f5d7c3c43da360e812b376e6bb0.

Reason for revert: [sheriff] Seeing failures such as these https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-dbg/9195

Original change's description:
> Navigation Predictor: Preconnect on tab foreground
> 
> When the tab is brought to the foreground, preconnect to the
> origin of the main frame request. This preconnection is done
> at most once, and is helpful in case user clicks on a link.
> 
> Change-Id: I6786c14ad7be0921bb309aabac5a75cff9819db8
> Bug: 908725
> Reviewed-on: https://chromium-review.googlesource.com/c/1350209
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#611107}

TBR=tbansal@chromium.org,ryansturm@chromium.org

Change-Id: If108b83423fd3db0a673ea70c4f6b226e6288406
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1351946
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#611156}
[modify] https://crrev.com/b195e066366cd83bd4f6abc4430ad31fede89895/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/b195e066366cd83bd4f6abc4430ad31fede89895/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/b195e066366cd83bd4f6abc4430ad31fede89895/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[modify] https://crrev.com/b195e066366cd83bd4f6abc4430ad31fede89895/tools/metrics/histograms/enums.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 27

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

commit acd1651b117f41ca27e77e605ba229e008992692
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Nov 27 22:49:50 2018

Navigation Predictor: Preconnect on tab foreground

When the tab is brought to the foreground, preconnect to the
origin of the main frame request. This preconnection is done
at most once, and is helpful in case user clicks on a link.

Bug: 908725
Change-Id: Ic4c35c881e6b1a99bdccaac1bbfd585cfab8cb48
Reviewed-on: https://chromium-review.googlesource.com/c/1352490
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611389}
[modify] https://crrev.com/acd1651b117f41ca27e77e605ba229e008992692/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/acd1651b117f41ca27e77e605ba229e008992692/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/acd1651b117f41ca27e77e605ba229e008992692/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[modify] https://crrev.com/acd1651b117f41ca27e77e605ba229e008992692/tools/metrics/histograms/enums.xml

Status: Fixed (was: Started)
Status: Started (was: Fixed)
The last patch did not do the work of preconnecting when app is brought to foreground. So, reopening this.
Labels: Merge-Request-72
Merge request for CL in #6.
Project Member

Comment 8 by sheriffbot@chromium.org, Dec 5

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
How is the change listed at #7 looking in canary? Is it safe to merge now?
https://chromiumdash.appspot.com/commit/6547f96874cbe397129e3c033fce43b4c0e24d07 shows it has not been released to Android. I can guess I will request merge approval tomorrow.
Labels: -Hotlist-Merge-Review -Merge-Review-72
NextAction: 2018-12-06
The NextAction date has arrived: 2018-12-06
NextAction: 2018-12-10
The change went in 73.0.3631.0 which has not been on Androidn Canary yet. Lets wait for Monday.
The NextAction date has arrived: 2018-12-10
Labels: Merge-Request-72
Project Member

Comment 17 by sheriffbot@chromium.org, Dec 10

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 18 by bugdroid1@chromium.org, Dec 10

How is the change listed at #7 looking in canary? Is it safe to merge now?

Also is cl listed at #18 need a merge to M72 once baked/verified in canary?
Yes, both of them would need to be merged. For now, I am asking for merge request for CL in #6 which went in 73.0.3631.0. That CL is doing well in canary.
Labels: -Merge-Review-72 Merge-Approved-72
Approving merge for CL listed at #6 to M72 branch 3626 based on comment #20. Please merge ASAP.

Request merge for CL listed at #18 after canary baking. 
Project Member

Comment 22 by bugdroid1@chromium.org, Dec 10

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c5b6771181fbb2f1bb9a005e805caa8e59c11bf

commit 9c5b6771181fbb2f1bb9a005e805caa8e59c11bf
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Dec 10 18:29:45 2018

Navigation Predictor: Take action on app foreground

Change-Id: If70228295d8509f9a95d2cae6436ca302ee3396f
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1357357
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613428}(cherry picked from commit 6547f96874cbe397129e3c033fce43b4c0e24d07)
Reviewed-on: https://chromium-review.googlesource.com/c/1370312
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#214}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/9c5b6771181fbb2f1bb9a005e805caa8e59c11bf/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/9c5b6771181fbb2f1bb9a005e805caa8e59c11bf/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/9c5b6771181fbb2f1bb9a005e805caa8e59c11bf/tools/metrics/histograms/enums.xml

NextAction: 2018-12-12
Pls update bug with canary result on Wednesday and request a merge for cl listed at #18 if change looks good in canary.
The NextAction date has arrived: 2018-12-12
Labels: -Hotlist-Merge-Review -merge-merged-3626 Merge-Request-72
Requesting merge for CL in #18. The CL went in Android 73.0.3637.0 which has been in canary for couple of days.
Project Member

Comment 26 by sheriffbot@chromium.org, Dec 12

Labels: -Merge-Request-72 Merge-Review-72 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-72 Merge-Approved-72
Approving merge to M72 branch 3626 for cl listed at #18 based on comment #25. 
Project Member

Comment 28 by bugdroid1@chromium.org, Dec 13

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d4ab5a1b148166fd909915a90c1ecc6396f11220

commit d4ab5a1b148166fd909915a90c1ecc6396f11220
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Dec 13 01:23:35 2018

Preconnect to the main frame origin

Logic to preconnect to the main frame origin when
app or tab is brought to the foreground.

The preconnect action is behind a finch trial, and is disabled
by default.

Change-Id: I0fb183d75997c702585b294da59016131b254683
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1365076
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615163}(cherry picked from commit 1e0b2e8a2f8490de9515c73120b16670ba17b30f)
Reviewed-on: https://chromium-review.googlesource.com/c/1374967
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#317}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/d4ab5a1b148166fd909915a90c1ecc6396f11220/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/d4ab5a1b148166fd909915a90c1ecc6396f11220/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/d4ab5a1b148166fd909915a90c1ecc6396f11220/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[modify] https://crrev.com/d4ab5a1b148166fd909915a90c1ecc6396f11220/chrome/browser/navigation_predictor/navigation_predictor_unittest.cc
[modify] https://crrev.com/d4ab5a1b148166fd909915a90c1ecc6396f11220/chrome/browser/predictors/loading_predictor_config.h

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 17

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

commit ce2ff3b6610aa28af22b34ea6c6fc0283bdbd924
Author: David Roger <droger@chromium.org>
Date: Mon Dec 17 10:15:27 2018

Revert "Preconnect to the main frame origin"

This reverts commit 1e0b2e8a2f8490de9515c73120b16670ba17b30f.

Reason for revert:
NavigationPredictorBrowserTest.ActionAccuracy_DifferentOrigin_VisibilityChanged is flaky
Bug:  915526 

Original change's description:
> Preconnect to the main frame origin
> 
> Logic to preconnect to the main frame origin when
> app or tab is brought to the foreground.
> 
> The preconnect action is behind a finch trial, and is disabled
> by default.
> 
> Change-Id: I0fb183d75997c702585b294da59016131b254683
> Bug: 908725
> Reviewed-on: https://chromium-review.googlesource.com/c/1365076
> Reviewed-by: Egor Pasko <pasko@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#615163}

TBR=pasko@chromium.org,tbansal@chromium.org,ryansturm@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 908725
Change-Id: I2aa0d83d1ebb56cc4539d3a575fae66856d69983
Reviewed-on: https://chromium-review.googlesource.com/c/1379757
Reviewed-by: David Roger <droger@chromium.org>
Commit-Queue: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617073}
[modify] https://crrev.com/ce2ff3b6610aa28af22b34ea6c6fc0283bdbd924/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/ce2ff3b6610aa28af22b34ea6c6fc0283bdbd924/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/ce2ff3b6610aa28af22b34ea6c6fc0283bdbd924/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[modify] https://crrev.com/ce2ff3b6610aa28af22b34ea6c6fc0283bdbd924/chrome/browser/navigation_predictor/navigation_predictor_unittest.cc
[modify] https://crrev.com/ce2ff3b6610aa28af22b34ea6c6fc0283bdbd924/chrome/browser/predictors/loading_predictor_config.h

Project Member

Comment 30 by bugdroid1@chromium.org, Dec 17

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

commit 7fee7b25e03c1a8181dc32d270d3ea2c505941ee
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Dec 17 20:53:18 2018

Reland Preconnect to the main frame origin

Logic to preconnect to the main frame origin when
app or tab is brought to the foreground.

The preconnect action is behind a finch trial, and is disabled
by default.

This is a reland of the CL which got reverted due to
flaky browsertest.

Bug: 908725
Change-Id: I0005fb06d3d1cc90ef1fd1f0a9a475325782363a
TBR: pasko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1380336
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617219}
[modify] https://crrev.com/7fee7b25e03c1a8181dc32d270d3ea2c505941ee/chrome/browser/navigation_predictor/navigation_predictor.cc
[modify] https://crrev.com/7fee7b25e03c1a8181dc32d270d3ea2c505941ee/chrome/browser/navigation_predictor/navigation_predictor.h
[modify] https://crrev.com/7fee7b25e03c1a8181dc32d270d3ea2c505941ee/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[modify] https://crrev.com/7fee7b25e03c1a8181dc32d270d3ea2c505941ee/chrome/browser/navigation_predictor/navigation_predictor_unittest.cc
[modify] https://crrev.com/7fee7b25e03c1a8181dc32d270d3ea2c505941ee/chrome/browser/predictors/loading_predictor_config.h

Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/9c5b6771181fbb2f1bb9a005e805caa8e59c11bf

Commit: 9c5b6771181fbb2f1bb9a005e805caa8e59c11bf
Author: tbansal@chromium.org
Commiter: tbansal@chromium.org
Date: 2018-12-10 18:29:45 +0000 UTC

Navigation Predictor: Take action on app foreground

Change-Id: If70228295d8509f9a95d2cae6436ca302ee3396f
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1357357
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#613428}(cherry picked from commit 6547f96874cbe397129e3c033fce43b4c0e24d07)
Reviewed-on: https://chromium-review.googlesource.com/c/1370312
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#214}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/d4ab5a1b148166fd909915a90c1ecc6396f11220

Commit: d4ab5a1b148166fd909915a90c1ecc6396f11220
Author: tbansal@chromium.org
Commiter: tbansal@chromium.org
Date: 2018-12-13 01:23:35 +0000 UTC

Preconnect to the main frame origin

Logic to preconnect to the main frame origin when
app or tab is brought to the foreground.

The preconnect action is behind a finch trial, and is disabled
by default.

Change-Id: I0fb183d75997c702585b294da59016131b254683
Bug: 908725
Reviewed-on: https://chromium-review.googlesource.com/c/1365076
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#615163}(cherry picked from commit 1e0b2e8a2f8490de9515c73120b16670ba17b30f)
Reviewed-on: https://chromium-review.googlesource.com/c/1374967
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#317}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Project Member

Comment 33 by bugdroid1@chromium.org, Dec 21

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

commit 6c4f0b174c34ceb370932afb49bc6bc21bd3f896
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Dec 21 20:19:06 2018

Fix compile error in navigation predictor browsertest in branch 3626.


Reland Preconnect to the main frame origin

Logic to preconnect to the main frame origin when
app or tab is brought to the foreground.

The preconnect action is behind a finch trial, and is disabled
by default.

This is a reland of the CL which got reverted due to
flaky browsertest.

(cherry picked from commit 7fee7b25e03c1a8181dc32d270d3ea2c505941ee)

Bug: 908725, 917283
Change-Id: I0005fb06d3d1cc90ef1fd1f0a9a475325782363a
TBR: pasko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1380336
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#617219}
Reviewed-on: https://chromium-review.googlesource.com/c/1388718
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#501}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/6c4f0b174c34ceb370932afb49bc6bc21bd3f896/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc

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

Commit: 6c4f0b174c34ceb370932afb49bc6bc21bd3f896
Author: tbansal@chromium.org
Commiter: tbansal@chromium.org
Date: 2018-12-21 20:19:06 +0000 UTC

Fix compile error in navigation predictor browsertest in branch 3626.


Reland Preconnect to the main frame origin

Logic to preconnect to the main frame origin when
app or tab is brought to the foreground.

The preconnect action is behind a finch trial, and is disabled
by default.

This is a reland of the CL which got reverted due to
flaky browsertest.

(cherry picked from commit 7fee7b25e03c1a8181dc32d270d3ea2c505941ee)

Bug: 908725, 917283
Change-Id: I0005fb06d3d1cc90ef1fd1f0a9a475325782363a
TBR: pasko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1380336
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Egor Pasko <pasko@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#617219}
Reviewed-on: https://chromium-review.googlesource.com/c/1388718
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#501}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
Blockedon: 919566

Sign in to add a comment