New issue
Advanced search Search tips

Issue 899339 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 5
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug

Blocking:
issue 850624



Sign in to add a comment

NavigationPredictor: Skip anchor elements that belong to an ad frame

Project Member Reported by tbansal@chromium.org, Oct 26

Issue description

At OnLoad event, currently, the renderer notifies browser process of the
anchor elements in the viewport. The browser uses this information
to predict the navigation probability of each of these anchor
elements.

Some of these anchor elements may belong to ad frames. For banner ads etc., these anchor elements may take a lot of space on the screen. This messes up with our prediction algorithms.

For now, the render can skip sending anchor elements to browser process that belong to ad frames since there are no concrete plans to use them on the browser side. This might also help with performance issues (See  Issue 893855  and  Issue 893864 ).
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 1

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

commit 36713b504ce73cffb00f386ff1e33c4c3fe03f34
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Nov 01 22:58:01 2018

NavigationPredictor: Skip anchor elements that belong to an ad frame

At OnLoad event, the renderer notifies browser process of the
anchor elements in the viewport. The browser uses this information
to predict the navigation probability of each of these anchor
elements.

This CL changes the logic in the render side to skip reporting
of elements that are part of ad frames since we expect these
to be clicked less often.

Change-Id: Ia131ac301acda317e7618ac206f4bda60c39e5cf
Bug:  899339 
Reviewed-on: https://chromium-review.googlesource.com/c/1300682
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604738}
[modify] https://crrev.com/36713b504ce73cffb00f386ff1e33c4c3fe03f34/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[add] https://crrev.com/36713b504ce73cffb00f386ff1e33c4c3fe03f34/chrome/test/data/navigation_predictor/iframe_ads_simple_page_with_anchors.html
[add] https://crrev.com/36713b504ce73cffb00f386ff1e33c4c3fe03f34/chrome/test/data/navigation_predictor/page_with_ads_iframe.html
[modify] https://crrev.com/36713b504ce73cffb00f386ff1e33c4c3fe03f34/third_party/blink/renderer/core/html/anchor_element_metrics_sender.cc
[modify] https://crrev.com/36713b504ce73cffb00f386ff1e33c4c3fe03f34/tools/metrics/histograms/histograms.xml

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 2

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

commit 5a3040f98281d6280e1017d14ef28e67dfe7cc4c
Author: Juan Antonio Navarro Pérez <perezju@chromium.org>
Date: Fri Nov 02 14:32:26 2018

Revert "NavigationPredictor: Skip anchor elements that belong to an ad frame"

This reverts commit 36713b504ce73cffb00f386ff1e33c4c3fe03f34.

Reason for revert: Causes Chrome to crash on power.typical_10_mobile Rotating earth test.

Original change's description:
> NavigationPredictor: Skip anchor elements that belong to an ad frame
> 
> At OnLoad event, the renderer notifies browser process of the
> anchor elements in the viewport. The browser uses this information
> to predict the navigation probability of each of these anchor
> elements.
> 
> This CL changes the logic in the render side to skip reporting
> of elements that are part of ad frames since we expect these
> to be clicked less often.
> 
> Change-Id: Ia131ac301acda317e7618ac206f4bda60c39e5cf
> Bug:  899339 
> Reviewed-on: https://chromium-review.googlesource.com/c/1300682
> Reviewed-by: Steven Holte <holte@chromium.org>
> Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#604738}

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

Change-Id: Ifa21f582007d7f50917e66a94ea905cf3a095902
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  899339 
Reviewed-on: https://chromium-review.googlesource.com/c/1315208
Reviewed-by: Juan Antonio Navarro Pérez <perezju@chromium.org>
Commit-Queue: Juan Antonio Navarro Pérez <perezju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604911}
[modify] https://crrev.com/5a3040f98281d6280e1017d14ef28e67dfe7cc4c/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[delete] https://crrev.com/1d064e771eff76425807c1efd3908b395954b453/chrome/test/data/navigation_predictor/iframe_ads_simple_page_with_anchors.html
[delete] https://crrev.com/1d064e771eff76425807c1efd3908b395954b453/chrome/test/data/navigation_predictor/page_with_ads_iframe.html
[modify] https://crrev.com/5a3040f98281d6280e1017d14ef28e67dfe7cc4c/third_party/blink/renderer/core/html/anchor_element_metrics_sender.cc
[modify] https://crrev.com/5a3040f98281d6280e1017d14ef28e67dfe7cc4c/tools/metrics/histograms/histograms.xml

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 2

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

commit 613d4a078d61410d43573897657b2c8cad546df5
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Nov 02 19:27:51 2018

NavigationPredictor: Skip anchor elements that belong to an ad frame

At OnLoad event, the renderer notifies browser process of the
anchor elements in the viewport. The browser uses this information
to predict the navigation probability of each of these anchor
elements.

This CL changes the logic in the render side to skip reporting
of elements that are part of ad frames since we expect these
to be clicked less often.

Bug:  899339 
Change-Id: I171cce5d4b9f722039222865753f3b7cbe6d8a79
TBR: holte@chromium.org,ryansturm@chromium.org,kinuko@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1315529
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605013}
[modify] https://crrev.com/613d4a078d61410d43573897657b2c8cad546df5/chrome/browser/navigation_predictor/navigation_predictor_browsertest.cc
[add] https://crrev.com/613d4a078d61410d43573897657b2c8cad546df5/chrome/test/data/navigation_predictor/iframe_ads_simple_page_with_anchors.html
[add] https://crrev.com/613d4a078d61410d43573897657b2c8cad546df5/chrome/test/data/navigation_predictor/page_with_ads_iframe.html
[modify] https://crrev.com/613d4a078d61410d43573897657b2c8cad546df5/third_party/blink/renderer/core/html/anchor_element_metrics_sender.cc
[modify] https://crrev.com/613d4a078d61410d43573897657b2c8cad546df5/tools/metrics/histograms/histograms.xml

Status: Fixed (was: Started)

Sign in to add a comment