chrome/common/ntp_logging_events.h - Issue 2117373002: Cleanup: Change LogMostVisitedImpression|Navigation APIs to take an enum

Side by Side Diff: chrome/common/ntp_logging_events.h

Issue 2117373002: Cleanup: Change LogMostVisitedImpression|Navigation APIs to take an enum (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@ntp_uma_cleanup

Patch Set: TODO Created 4 years, 5 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

« no previous file with comments | « chrome/browser/ui/webui/ntp/ntp_user_data_logger_unittest.cc ('k') | chrome/common/render_messages.h » ('j') | chrome/renderer/searchbox/searchbox_extension.cc » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 // Copyright 2013 The Chromium Authors. All rights reserved.	1 // Copyright 2013 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_COMMON_NTP_LOGGING_EVENTS_H_	5 #ifndef CHROME_COMMON_NTP_LOGGING_EVENTS_H_

6 #define CHROME_COMMON_NTP_LOGGING_EVENTS_H_	6 #define CHROME_COMMON_NTP_LOGGING_EVENTS_H_

7	7

8 // The different types of events that are logged from the NTP. This enum is used	8 // The different types of events that are logged from the NTP. This enum is used

9 // to transfer information from the NTP javascript to the renderer and is not	9 // to transfer information from the NTP javascript to the renderer and is not

10 // used as a UMA enum histogram's logged value.	10 // used as a UMA enum histogram's logged value.

(...skipping 31 matching lines...) Expand 10 before \| Expand all \| Expand 10 after Loading...
42	42

43 // The user moused over an NTP tile or title.	43 // The user moused over an NTP tile or title.

44 NTP_MOUSEOVER = 9,	44 NTP_MOUSEOVER = 9,

45	45

46 // A NTP Tile has finished loading (successfully or failing).	46 // A NTP Tile has finished loading (successfully or failing).

47 NTP_TILE_LOADED = 10,	47 NTP_TILE_LOADED = 10,

48	48

49 NTP_EVENT_TYPE_LAST = NTP_TILE_LOADED	49 NTP_EVENT_TYPE_LAST = NTP_TILE_LOADED

50 };	50 };

51	51

	52 // The source of an NTP tile.

	53 // Note: Keep in sync with browser/resources/local_ntp/most_visited_util.js and

	54 // browser/resources/local_ntp/most_visited_single.js.

	55 // TODO(treib): Merge this into MostVisitedSource from components/ntp_tiles.

	56 enum NTPLoggingTileSource {
	dcheng 2016/07/06 01:51:25 Nit: Can we make this a scoped enum, i.e. enum cla Nit: Can we make this a scoped enum, i.e. enum class NTPLoggingTileSource? Then the enum members can just be CLIENT, SERVER, LAST. Marc Treib 2016/07/06 08:49:06 The enum values are duplicated in JavaScript code. Show quoted text On 2016/07/06 01:51:25, dcheng wrote: > Nit: Can we make this a scoped enum, i.e. enum class NTPLoggingTileSource? Then > the enum members can just be CLIENT, SERVER, LAST. The enum values are duplicated in JavaScript code. To at least make them greppable, I'd like to keep the long, unique names. (If you know of a way to share an enum (or just some constants) between C++ and JS, I'd love to hear it...) Bernhard Bauer 2016/07/06 09:00:51 BUT I WANTED TO USE THE SHORT VALUES IN JAVASCRIPT Show quoted text On 2016/07/06 08:49:06, Marc Treib wrote: > On 2016/07/06 01:51:25, dcheng wrote: > > Nit: Can we make this a scoped enum, i.e. enum class NTPLoggingTileSource? > Then > > the enum members can just be CLIENT, SERVER, LAST. > > The enum values are duplicated in JavaScript code. To at least make them > greppable, I'd like to keep the long, unique names. BUT I WANTED TO USE THE SHORT VALUES IN JAVASCRIPT AS WELL 😉 If we always use them namespaced (which we have to do anyway), grepping should be possible too, no? You'd just have to use different namespace separators for JS vs C++. Show quoted text > (If you know of a way to share an enum (or just some constants) between C++ and > JS, I'd love to hear it...) Marc Treib 2016/07/06 10:14:38 Which is still a lot less convenient (and thus mor Show quoted text On 2016/07/06 09:00:51, Bernhard Bauer wrote: > On 2016/07/06 08:49:06, Marc Treib wrote: > > On 2016/07/06 01:51:25, dcheng wrote: > > > Nit: Can we make this a scoped enum, i.e. enum class NTPLoggingTileSource? > > Then > > > the enum members can just be CLIENT, SERVER, LAST. > > > > The enum values are duplicated in JavaScript code. To at least make them > > greppable, I'd like to keep the long, unique names. > > BUT I WANTED TO USE THE SHORT VALUES IN JAVASCRIPT AS WELL 😉 > > If we always use them namespaced (which we have to do anyway), grepping should > be possible too, no? You'd just have to use different namespace separators for > JS vs C++. > > > (If you know of a way to share an enum (or just some constants) between C++ > and > > JS, I'd love to hear it...) > Which is still a lot less convenient (and thus more error-prone) than just searching for one string. But okay, I guess I'm outvoted here. Done.
	57 NTP_TILE_SOURCE_CLIENT = 0,

	58 NTP_TILE_SOURCE_SERVER = 1,

	59 NTP_TILE_SOURCE_LAST = NTP_TILE_SOURCE_SERVER

	60 };

	61

52 #endif // CHROME_COMMON_NTP_LOGGING_EVENTS_H_	62 #endif // CHROME_COMMON_NTP_LOGGING_EVENTS_H_

OLD	NEW