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

Issue 712854 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit 22 days ago
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Page transition type on background loader should be explicit rather than linked

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

Issue description

From carlosk@, PAGE_TRANSITION_LINK implies render whereas PAGE_TRANSITION_TYPED is for all "explicit navigation" types:

https://cs.chromium.org/chromium/src/ui/base/page_transition_types.h?type=cs&sq=package:chromium&l=36
 
Pasting comment from the code review to kick-start the discussion here.

I am lacking a good description of a thought process that led us here:
BO is used when user:
* clicked a download button,
* clicked a button on dino page,
* clicked a context link,
all of which violate this:
"""
This should not be used for cases where the user selected a choice that didn't
look at all a URL; see GENERATED below.
"""
part of documentation of PAGE_TRANSITION_TYPED.
My initial intent was to move away from the LINK transition as it implies many things inside Chrome, more especially the existence of a renderer process where the link click/tap happened, which is never the case for background loader (can you confirm this chili@?).

I suggested TYPED as it is better suited in that sense but indeed it brings other assumptions that I incorrectly ignored.

Looking at the prerender code [1] it bases the choice of transition based on the what triggered the navigation, what is referred to as the "prerender origin" [2]. The latest change to this code due to  issue 345953  explains why TYPED was not well suited in some situations for prerender.

We should probably follow a similar pattern and take into consideration that for background loader all navigations are made with user intent (differently from the prerender case).

[1] https://cs.chromium.org/chromium/src/chrome/browser/prerender/prerender_contents.cc?type=cs&l=332-341

[2] https://cs.chromium.org/chromium/src/chrome/browser/prerender/prerender_origin.h

Comment 3 by chili@chromium.org, Apr 19 2017

@Carlosk: Background loader is attached to a render process that performs the rendering. The viewport is set to 0x0 and we are never visible, so the user would never have a chance to "click" on anything. I assumed that this is a reason why TYPED was suited.

The main reason I see for TYPED:
- disregarding the original user intent, the web contents where the loading happens is fresh, and we are loading a saved URL fresh. Part of the documentation for PAGE_TRANSITION_TYPED says that it can be used for "explicit" navigation actions, which I consider this to be.
- although documentation says "should not be used for cases where user-selected choice didn't look at all a URL", we are not actually downloading the dino page. The URL we are actually downloading is assumed to not be a chrome-URL


Reason for not TYPED:
- Perhaps we should preserve how user ended up in a situation where background loading was triggered (either they typed URL and landed on dino page or long-clicked on link to download or clicked link and landed on dino page). This may reflect better in omnibox rankings/any other places where this will have an effect

Comment 4 by chili@chromium.org, Jun 26 2017

Status: Available (was: Started)
I think in a best-case scenario, we want a background-specific page transition type
Status: Assigned (was: Available)
Suggest closing this, maybe not worth fixing.

Sign in to add a comment