NonValidatingReload: Monitor reload operations in NavigationControllerImpl

content/browser/frame_host/navigation_controller_impl.cc

 // Copyright 2013 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.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 223 matching lines @@
       pending_entry_index_(-1),
       transient_entry_index_(-1),
       delegate_(delegate),
       max_restored_page_id_(-1),
       ssl_manager_(this),
       needs_reload_(false),
       is_initial_navigation_(true),
       in_navigate_to_pending_entry_(false),
       pending_reload_(NO_RELOAD),
       get_timestamp_callback_(base::Bind(&base::Time::Now)),
-      screenshot_manager_(new NavigationEntryScreenshotManager(this)) {
+      screenshot_manager_(new NavigationEntryScreenshotManager(this)),
+      is_last_navigation_reload_(false),
+      last_reload_type_(NO_RELOAD) {
   DCHECK(browser_context_);
 }
 
 NavigationControllerImpl::~NavigationControllerImpl() {
   DiscardNonCommittedEntriesInternal();
 }
 
 WebContents* NavigationControllerImpl::GetWebContents() const {
   return delegate_->GetWebContents();
 }
@@ skipping 82 matching lines @@
     if (current_index != -1) {
       entry = GetEntryAtIndex(current_index);
     }
   }
 
   // If we are no where, then we can't reload.  TODO(darin): We should add a
   // CanReload method.
   if (!entry)
     return;
 
+  entry->set_reload_type(reload_type);

[Charlie Reis, 2016/08/08 23:57:53]

What happens if the renderer initiates a reload (e.g., location.reload() in
Javascript)?  That won't go through here, will it?

Maybe there's a parameter we can check at commit time instead of storing the bit
on NavigationEntry?

[Takashi Toyoshima, 2016/08/09 07:35:26]

Right. Javascript initiated reload won't be captured here. It directly calls
FrameLoader::load() with FrameLoadTypeReload from ScheduleReload::fire().

But reloads I'm interested here are ones come from user interfaces. It's fine
that JavaScript initiated reload is ignored. Probably, I should update comments
to clarify that.

If we want to capture the JavaScript initiated reloads too, probably we can
track it in RendererDidNavigate() as one of NAVIGATION_TYPE_EXISTING_PAGE, and
entry_index and url being not changed?

Another problem I noticed here is stored reload type isn't updated on JavaScript
initiated reload. That means if I follow steps below, it mistakenly report
ReloadToRealodDuration metric.

 1. Navigate to a page
 2. Reload user action
 3. location.reload() => stored information isn't updated, and entry still has
the previous reload_type. this results in reporting ReloadToReloadDuration
metric.

To fix this issue, I should identify JavaScript initiated reloads and user
actions initiated reloads. ClientRedirectPolicy in Blink is the one I can use
for that. But it isn't available here. Shall we plumb it from FrameLoader on
CommitLoad timing?

[Charlie Reis, 2016/08/10 05:05:13]

Isn't this a problem for any renderer-initiated navigation, not just
location.reload?  I'm still concerned about the overall strategy.

[Takashi Toyoshima, 2016/08/10 06:34:15]

If we assume this reload type is valid only when the page transition type
indicates reload, it does not matter. My proposed way, using
ClientRedirectPolicy, just works becase as far as I know, all JavaScript
initiated reload-ish navigation are classified to "clientRedirect".

+
   if (g_check_for_repost && check_for_repost &&
       entry->GetHasPostData()) {
     // The user is asking to reload a page with POST data. Prompt to make sure
     // they really want to do this. If they do, the dialog will call us back
     // with check_for_repost = false.
     delegate_->NotifyBeforeFormRepostWarningShow();
 
     pending_reload_ = reload_type;
     delegate_->ActivateAndShowRepostFormWarningDialog();
   } else {
@@ skipping 1581 matching lines @@
   }
 
   for (size_t i = 0; i < frame->child_count(); i++) {
     FindFramesToNavigate(frame->child_at(i), same_document_loads,
                          different_document_loads);
   }
 }
 
 void NavigationControllerImpl::NotifyNavigationEntryCommitted(
     LoadCommittedDetails* details) {
-  details->entry = GetLastCommittedEntry();
+  NavigationEntryImpl* entry = GetLastCommittedEntry();
+  CHECK(entry);
+  details->entry = entry;
+
+  ui::PageTransition transition = details->entry->GetTransitionType();
+  if (ui::PageTransitionIsMainFrame(transition)) {
+    base::Time now = base::Time::Now();
+
+    bool is_navigation_forward_back =
+        transition & ui::PAGE_TRANSITION_FORWARD_BACK;

[Takashi Toyoshima, 2016/08/09 12:06:17]

This line broke Windows build.

+    bool is_navigation_reload =
+        !is_navigation_forward_back &&
+        ui::PageTransitionCoreTypeIs(transition, ui::PAGE_TRANSITION_RELOAD);
+
+    if (is_last_navigation_reload_ && is_navigation_reload) {
+      base::TimeDelta delta = now - last_navigation_time_;
+      UMA_HISTOGRAM_MEDIUM_TIMES("Navigation.Reload.ReloadToReloadDuration",
+                                 delta);
+      // Track the case that another reload folllows RELOAD_MAIN_RESOURCE type
+      // of reload.
+      if (last_reload_type_ == NavigationController::RELOAD_MAIN_RESOURCE) {
+        UMA_HISTOGRAM_MEDIUM_TIMES(
+            "Navigation.Reload.ReloadMainResourceToReloadDuration", delta);
+      }
+    }
+
+    is_last_navigation_reload_ = is_navigation_reload;
+    last_reload_type_ = entry->reload_type();
+    last_navigation_time_ = now;
+  }
 
   // We need to notify the ssl_manager_ before the web_contents_ so the
   // location bar will have up-to-date information about the security style
   // when it wants to draw.  See http://crbug.com/11157
   ssl_manager_.DidCommitProvisionalLoad(*details);
 
   delegate_->NotifyNavigationStateChanged(INVALIDATE_TYPE_ALL);
   delegate_->NotifyNavigationEntryCommitted(*details);
 
   // TODO(avi): Remove. http://crbug.com/170921
@@ skipping 145 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content