chrome/browser/prerender/prerender_origin.h - Issue 1767243002: Update prerender policy for custom tabs

Side by Side Diff: chrome/browser/prerender/prerender_origin.h

Issue 1767243002: Update prerender policy for custom tabs (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Dan's comments Created 4 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« chrome/browser/prerender/prerender_manager.cc ('K') | « chrome/browser/prerender/prerender_manager.cc ('k') | chrome/browser/prerender/prerender_origin.cc » ('j') | chrome/browser/prerender/prerender_origin.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.	1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.

2 // Use of this source code is governed by a BSD-style license that can be	2 // Use of this source code is governed by a BSD-style license that can be

3 // found in the LICENSE file.	3 // found in the LICENSE file.

4	4

5 #ifndef CHROME_BROWSER_PRERENDER_PRERENDER_ORIGIN_H_	5 #ifndef CHROME_BROWSER_PRERENDER_PRERENDER_ORIGIN_H_

6 #define CHROME_BROWSER_PRERENDER_PRERENDER_ORIGIN_H_	6 #define CHROME_BROWSER_PRERENDER_PRERENDER_ORIGIN_H_

7	7

8 namespace prerender {	8 namespace prerender {

9	9

10 // Origin indicates what caused the prerender.	10 // Origin indicates what caused the prerender.

11 // NOTE: New values need to be appended, since they are used in histograms.	11 // NOTE: New values need to be appended, since they are used in histograms.

12 enum Origin {	12 enum Origin {

13 // Obsolete: ORIGIN_LINK_REL_PRERENDER = 0,	13 // Obsolete: ORIGIN_LINK_REL_PRERENDER = 0,

14 // Obsolete: ORIGIN_OMNIBOX_ORIGINAL = 1,	14 // Obsolete: ORIGIN_OMNIBOX_ORIGINAL = 1,

15 ORIGIN_GWS_PRERENDER = 2,	15 ORIGIN_GWS_PRERENDER = 2,

16 // Obsolete: ORIGIN_OMNIBOX_CONSERVATIVE = 3,	16 // Obsolete: ORIGIN_OMNIBOX_CONSERVATIVE = 3,

17 // Obsolete: ORIGIN_OMNIBOX_EXACT = 4,	17 // Obsolete: ORIGIN_OMNIBOX_EXACT = 4,

18 ORIGIN_OMNIBOX = 5,	18 ORIGIN_OMNIBOX = 5,

19 ORIGIN_NONE = 6,	19 ORIGIN_NONE = 6,

20 ORIGIN_LINK_REL_PRERENDER_SAMEDOMAIN = 7,	20 ORIGIN_LINK_REL_PRERENDER_SAMEDOMAIN = 7,

21 ORIGIN_LINK_REL_PRERENDER_CROSSDOMAIN = 8,	21 ORIGIN_LINK_REL_PRERENDER_CROSSDOMAIN = 8,

22 // Obsolete: ORIGIN_LOCAL_PREDICTOR = 9,	22 // Obsolete: ORIGIN_LOCAL_PREDICTOR = 9,

23 ORIGIN_EXTERNAL_REQUEST = 10,	23 ORIGIN_EXTERNAL_REQUEST = 10,

24 ORIGIN_INSTANT = 11,	24 ORIGIN_INSTANT = 11,

25 ORIGIN_LINK_REL_NEXT = 12,	25 ORIGIN_LINK_REL_NEXT = 12,

	26 ORIGIN_EXTERNAL_REQUEST_MOBILE = 13,
	Benoit L 2016/03/10 13:58:30 I guess that you need to update places where this I guess that you need to update places where this enum is used. At least for the histograms: https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/pre... Fortunately, it doesn't seem to be used too widely, that is mainly for histograms (we may want to conflate this with EXTERNAL_REQUEST) and throttling (ditto?). Also, should the name reflect that we force the prerender even though we're on a mobile network? Maybe ORIGIN_EXTERNAL_REQUEST_FORCED_ON_MOBILE? pasko 2016/03/10 14:21:37 +1, also would need a new suffix in histograms.xml +1, also would need a new suffix in histograms.xml Maybe ORIGIN_EXTERNAL_REQUEST_FORCED, since the mechanics of PrerenderManager is not mobile-specific. On the other hand, it is important to emphasize in the origin what kind of hack it is. Maybe then RIGIN_EXTERNAL_REQUEST_FORCED_CUSTOMTABS is even more descriptive. The offline thing will probably use the same mechanics, but we would like the origin to be different for UMA accounting reasons. Yusuf 2016/03/10 18:55:53 Done. Show quoted text On 2016/03/10 14:21:37, pasko wrote: > +1, also would need a new suffix in histograms.xml > > Maybe ORIGIN_EXTERNAL_REQUEST_FORCED, since the mechanics of PrerenderManager is > not mobile-specific. On the other hand, it is important to emphasize in the > origin what kind of hack it is. Maybe then > RIGIN_EXTERNAL_REQUEST_FORCED_CUSTOMTABS is even more descriptive. The offline > thing will probably use the same mechanics, but we would like the origin to be > different for UMA accounting reasons. > Done. Yusuf 2016/03/10 18:55:53 Done. Show quoted text On 2016/03/10 14:21:37, pasko wrote: > +1, also would need a new suffix in histograms.xml > > Maybe ORIGIN_EXTERNAL_REQUEST_FORCED, since the mechanics of PrerenderManager is > not mobile-specific. On the other hand, it is important to emphasize in the > origin what kind of hack it is. Maybe then > RIGIN_EXTERNAL_REQUEST_FORCED_CUSTOMTABS is even more descriptive. The offline > thing will probably use the same mechanics, but we would like the origin to be > different for UMA accounting reasons. > Done.
26 ORIGIN_MAX,	27 ORIGIN_MAX,

27 };	28 };

28	29

29 // Return a human-readable name for \|origin\|.	30 // Return a human-readable name for \|origin\|.

30 const char* NameFromOrigin(Origin origin);	31 const char* NameFromOrigin(Origin origin);

31	32

32 } // namespace prerender	33 } // namespace prerender

33	34

34 #endif // CHROME_BROWSER_PRERENDER_PRERENDER_ORIGIN_H_	35 #endif // CHROME_BROWSER_PRERENDER_PRERENDER_ORIGIN_H_

OLD	NEW