Skip to content

Commit

Permalink
[New Tab Page] Don't record metrics if page destroyed
Browse files Browse the repository at this point in the history
It removes a minor inconsistency across UMA metrics, since some are
recorded always (code before early return), and others depending on
mIsDestroyed (below after early return).

It shouldn't make a big difference in practice due to how unlikely the
scenario is: the New Tab Page is closed while it is in the process of
loading (including icons, etc.).

BUG=

Review-Url: https://codereview.chromium.org/1965543005
Cr-Commit-Position: refs/heads/master@{#392892}
  • Loading branch information
mastiz authored and Commit bot committed May 11, 2016
1 parent 11513aa commit 1ea1ef4
Showing 1 changed file with 4 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -525,16 +525,17 @@ public void onLogoAvailable(Logo logo, boolean fromCache) {

@Override
public void onLoadingComplete(MostVisitedItem[] items) {
if (mIsDestroyed) return;

long loadTimeMs = (System.nanoTime() - mConstructedTimeNs) / 1000000;
RecordHistogram.recordTimesHistogram(
"Tab.NewTabOnload", loadTimeMs, TimeUnit.MILLISECONDS);
mIsLoaded = true;
// TODO(mastiz): revisit the logic for mIsLoaded and mIsVisible, since setting
// mIsVisible to true seems wrong here.
mIsVisible = true;
StartupMetrics.getInstance().recordOpenedNTP();
NewTabPageUma.recordNTPImpression(NewTabPageUma.NTP_IMPRESSION_REGULAR);

if (mIsDestroyed) return;

recordNTPShown();

int tileTypes[] = new int[items.length];
Expand Down

0 comments on commit 1ea1ef4

Please sign in to comment.