Desktop NTP metrics: Use ntp_tiles::metrics:: functions

components/ntp_tiles/metrics.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef COMPONENTS_NTP_TILES_METRICS_H_
 #define COMPONENTS_NTP_TILES_METRICS_H_
 
 #include <vector>
 
 #include "components/ntp_tiles/ntp_tile.h"
@@ skipping 13 matching lines @@
   NONE,
   // The item displays a site's actual favicon or touch icon.
   ICON_REAL,
   // The item displays a color derived from the site's favicon or touch icon.
   ICON_COLOR,
   // The item displays a default gray box in place of an icon.
   ICON_DEFAULT,
   NUM_TILE_TYPES,
 };
 
-// TODO(treib): Split into individual impressions for desktop.
-void RecordImpressions(const NTPTilesVector& tiles);
+// Records an impression of an individual tile.
+void RecordTileImpression(int index, NTPTileSource source);
 
+// Records an impression of the page itself, after all tiles have loaded.
+void RecordPageImpression(int number_of_tiles);
+
+// Records the visual types (see above) of all visible tiles.
 void RecordImpressionTileTypes(
     const std::vector<MostVisitedTileType>& tile_types,
     const std::vector<NTPTileSource>& sources);
 
-// TODO(treib): Split into "base" click and tile type for desktop.
-void RecordClick(int index,
-                 MostVisitedTileType tile_type,
-                 NTPTileSource source);
+// Records a click on a tile.
+void RecordTileClick(int index, NTPTileSource source);
+
+// Records a click on a tile, and also the type of the tile that was clicked.
+void RecordTileClickWithTileType(int index,
+                                 NTPTileSource source,
+                                 MostVisitedTileType tile_type);

[sfiera, 2016/10/25 12:12:11]

Rather than splitting out a separate function for Desktop, wouldn't it make more
sense to add ICON_THUMBNAIL to the tile type enum and use that?

[Marc Treib, 2016/10/25 12:23:39]

Hm. We could add it to the enum, but actually recording it doesn't really make
sense (it'll always be THUMBNAIL on desktop, and never on mobile). And then
RecordImpressionTileTypes would take one input that we'd never expect to
occur... hm. Might still be the better tradeoff? WDYT?

[sfiera, 2016/10/25 12:42:45]

I think I'd go for it. Personally, I think touch icons tend to be better than
thumbnails anyway and it might be worth experimenting with them on Desktop some
time in the future.

[Marc Treib, 2016/10/25 13:26:51]

Fun fact: A desktop icon NTP is actually mostly implemented - I can tell you
about it if you're interested :)

Anyway, I don't buy the "we might want this at some unspecified point in the
future" argument. If so, it's just a small CL away. Then again, I do like having
just one function for recording a click. So, done.

(In a later CL, I'll similarly merge RecordPageImpression and
RecordImpressionTileTypes.)

 
 }  // namespace metrics
 }  // namespace ntp_tiles
 
 #endif