Change NavigationHandle::GetRedirectChain to support server vs client redirects |
||||||
Issue description
From clamy@:
I think a first step would be to better document the NavigationHandle method, so that there is less surprise. When it comes to modifying the method, I'd prefer to provide callers with a method that allows them to specify which kind of redirects they want.
For example:
enum class RedirectType {
SERVER_ONLY,
CLIENT_ONLY,
CLIENT_AND_SERVER,
};
const std::vector<GURL>& GetRedirectChain(RedirectType redirect_type);
Once this is implemented, initially all call sites will user CLIENT_AND_SERVER except previews which will use SERVER_ONLY.
,
May 31 2017
,
Jun 1 2017
This gets complicated when trying to return a const vector& to parts of a vector. or a const vector& to a combination of two vectors. I'm not sure if there is something like StringPiece for containers in chromium, but it seems like this change could introduce some unneeded string copies.
,
Jun 8 2017
Ryan, would you mind adding a little more context to this issue so we can follow along?
,
Jun 8 2017
For context, the first URL in the chain was previously used to determine if a preview should be used, so when adding the URL to the previews blacklist, we should use that URL again (https://cs.chromium.org/chromium/src/chrome/browser/previews/previews_infobar_tab_helper.cc?type=cs&q=previews+getredirectchain&l=125). The context for PLM/UKM is counting the number of redirects since the start of the navigation, so that we can correlate the times based on redirect chain length. In both these cases, counting/using the first URL in the chain is sometimes correct sometimes not correct.
,
Nov 30 2017
Refreshed during triage.
,
Dec 5 2017
,
Dec 5 2017
,
Jan 24 2018
Refreshed during triage.
,
Mar 21 2018
,
May 15 2018
I think this will have to wait until after previews has gone through s13n to understand this problem more fully. Right now it is not a high priority.
,
Jun 22 2018
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by ryansturm@chromium.org
, May 31 2017