diff options
author | clamy@chromium.org <clamy@chromium.org> | 2015-01-29 16:03:18 +0100 |
---|---|---|
committer | clamy@chromium.org <clamy@chromium.org> | 2015-01-29 15:04:40 +0000 |
commit | 88420c2b893131a37c36a6d02319e866a032dbeb (patch) | |
tree | 96f7e4611f8310b211a3ae48b96aa8ff991b8746 | |
parent | f5117c891fe8f1a1851facaa957e41bb8777d6af (diff) | |
download | chromium_src-88420c2b893131a37c36a6d02319e866a032dbeb.zip chromium_src-88420c2b893131a37c36a6d02319e866a032dbeb.tar.gz chromium_src-88420c2b893131a37c36a6d02319e866a032dbeb.tar.bz2 |
Add metric for navigation starting from the time an intent was received
This CL adds two metrics that measure the time between receiving an Android
intent and the commit and completion of a navigation.
BUG=436955
Review URL: https://codereview.chromium.org/809043002
Cr-Commit-Position: refs/heads/master@{#312861}
(cherry picked from commit 8451aa6c32e3d0236266445aa10fab9424b1ac42)
Review URL: https://codereview.chromium.org/887703002
Cr-Commit-Position: refs/branch-heads/2272@{#153}
Cr-Branched-From: 827a380cfdb31aa54c8d56e63ce2c3fd8c3ba4d4-refs/heads/master@{#310958}
-rw-r--r-- | chrome/android/java/src/org/chromium/chrome/browser/Tab.java | 4 | ||||
-rw-r--r-- | chrome/browser/android/tab_android.cc | 4 | ||||
-rw-r--r-- | chrome/browser/android/tab_android.h | 3 | ||||
-rw-r--r-- | content/browser/frame_host/navigation_controller_impl.cc | 8 | ||||
-rw-r--r-- | content/browser/frame_host/navigation_entry_impl.cc | 6 | ||||
-rw-r--r-- | content/browser/frame_host/navigation_entry_impl.h | 18 | ||||
-rw-r--r-- | content/browser/frame_host/navigator_impl.cc | 24 | ||||
-rw-r--r-- | content/browser/frame_host/render_frame_host_impl.cc | 25 | ||||
-rw-r--r-- | content/common/frame_message_enums.h | 5 | ||||
-rw-r--r-- | content/common/frame_messages.h | 2 | ||||
-rw-r--r-- | content/common/navigation_params.cc | 16 | ||||
-rw-r--r-- | content/common/navigation_params.h | 12 | ||||
-rw-r--r-- | content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java | 16 | ||||
-rw-r--r-- | content/public/browser/navigation_controller.cc | 11 | ||||
-rw-r--r-- | content/public/browser/navigation_controller.h | 7 | ||||
-rw-r--r-- | content/renderer/render_frame_impl.cc | 22 | ||||
-rw-r--r-- | tools/metrics/histograms/histograms.xml | 16 |
17 files changed, 181 insertions, 18 deletions
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/Tab.java b/chrome/android/java/src/org/chromium/chrome/browser/Tab.java index 6236046..5131095 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/Tab.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/Tab.java @@ -764,7 +764,7 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener, // TODO(ppi): Should we pass Referrer jobject and add JNI methods to read it // from the native? params.getReferrer() != null ? params.getReferrer().getPolicy() : 0, - params.getIsRendererInitiated()); + params.getIsRendererInitiated(), params.getIntentReceivedTimestamp()); for (TabObserver observer : mObservers) { observer.onLoadUrl(this, params.getUrl(), loadType); @@ -2373,7 +2373,7 @@ public class Tab implements ViewGroup.OnHierarchyChangeListener, private native Profile nativeGetProfileAndroid(long nativeTabAndroid); private native int nativeLoadUrl(long nativeTabAndroid, String url, String extraHeaders, byte[] postData, int transition, String referrerUrl, int referrerPolicy, - boolean isRendererInitiated); + boolean isRendererInitiated, long intentReceivedTimestamp); private native void nativeSetActiveNavigationEntryTitleForUrl(long nativeTabAndroid, String url, String title); private native boolean nativePrint(long nativeTabAndroid); diff --git a/chrome/browser/android/tab_android.cc b/chrome/browser/android/tab_android.cc index e7ce1c8..840b3f8 100644 --- a/chrome/browser/android/tab_android.cc +++ b/chrome/browser/android/tab_android.cc @@ -499,7 +499,8 @@ TabAndroid::TabLoadStatus TabAndroid::LoadUrl(JNIEnv* env, jint page_transition, jstring j_referrer_url, jint referrer_policy, - jboolean is_renderer_initiated) { + jboolean is_renderer_initiated, + jlong intent_received_timestamp) { if (!web_contents()) return PAGE_LOAD_FAILED; @@ -581,6 +582,7 @@ TabAndroid::TabLoadStatus TabAndroid::LoadUrl(JNIEnv* env, return DEFAULT_PAGE_LOAD; } load_params.is_renderer_initiated = is_renderer_initiated; + load_params.intent_received_timestamp = intent_received_timestamp; web_contents()->GetController().LoadURLWithParams(load_params); } return DEFAULT_PAGE_LOAD; diff --git a/chrome/browser/android/tab_android.h b/chrome/browser/android/tab_android.h index d411470..57c019b 100644 --- a/chrome/browser/android/tab_android.h +++ b/chrome/browser/android/tab_android.h @@ -153,7 +153,8 @@ class TabAndroid : public CoreTabHelperDelegate, jint page_transition, jstring j_referrer_url, jint referrer_policy, - jboolean is_renderer_initiated); + jboolean is_renderer_initiated, + jlong intent_received_timestamp); void SetActiveNavigationEntryTitleForUrl(JNIEnv* env, jobject obj, jstring jurl, diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc index f4f184b..867466c 100644 --- a/content/browser/frame_host/navigation_controller_impl.cc +++ b/content/browser/frame_host/navigation_controller_impl.cc @@ -733,6 +733,14 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) { params.transferred_global_request_id); entry->SetFrameToNavigate(params.frame_name); +#if defined(OS_ANDROID) + if (params.intent_received_timestamp > 0) { + entry->set_intent_received_timestamp( + base::TimeTicks() + + base::TimeDelta::FromMilliseconds(params.intent_received_timestamp)); + } +#endif + switch (params.load_type) { case LOAD_TYPE_DEFAULT: break; diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc index 8176847..849871a 100644 --- a/content/browser/frame_host/navigation_entry_impl.cc +++ b/content/browser/frame_host/navigation_entry_impl.cc @@ -338,6 +338,12 @@ void NavigationEntryImpl::ResetForCommit() { set_should_clear_history_list(false); set_frame_tree_node_id(-1); set_source_site_instance(nullptr); + +#if defined(OS_ANDROID) + // Reset the time stamp so that the metrics are not reported if this entry is + // loaded again in the future. + set_intent_received_timestamp(base::TimeTicks()); +#endif } void NavigationEntryImpl::SetScreenshotPNGData( diff --git a/content/browser/frame_host/navigation_entry_impl.h b/content/browser/frame_host/navigation_entry_impl.h index 8505f5b..f458bc0 100644 --- a/content/browser/frame_host/navigation_entry_impl.h +++ b/content/browser/frame_host/navigation_entry_impl.h @@ -7,6 +7,7 @@ #include "base/basictypes.h" #include "base/memory/ref_counted.h" +#include "base/time/time.h" #include "content/browser/site_instance_impl.h" #include "content/public/browser/favicon_status.h" #include "content/public/browser/global_request_id.h" @@ -223,6 +224,17 @@ class CONTENT_EXPORT NavigationEntryImpl frame_tree_node_id_ = frame_tree_node_id; } +#if defined(OS_ANDROID) + base::TimeTicks intent_received_timestamp() const { + return intent_received_timestamp_; + } + + void set_intent_received_timestamp( + const base::TimeTicks intent_received_timestamp) { + intent_received_timestamp_ = intent_received_timestamp; + } +#endif + private: // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING // Session/Tab restore save portions of this class so that it can be recreated @@ -341,6 +353,12 @@ class CONTENT_EXPORT NavigationEntryImpl // TODO(creis): Move this to FrameNavigationEntry. int64 frame_tree_node_id_; +#if defined(OS_ANDROID) + // The time at which Chrome received the Android Intent that triggered this + // URL load operation. Reset at commit and not persisted. + base::TimeTicks intent_received_timestamp_; +#endif + // Used to store extra data to support browser features. This member is not // persisted, unless specific data is taken out/put back in at save/restore // time (see TabNavigation for an example of this). diff --git a/content/browser/frame_host/navigator_impl.cc b/content/browser/frame_host/navigator_impl.cc index a9a54b8..8ef9013 100644 --- a/content/browser/frame_host/navigator_impl.cc +++ b/content/browser/frame_host/navigator_impl.cc @@ -142,10 +142,19 @@ void MakeNavigateParams(const NavigationEntryImpl& entry, NavigationController::ReloadType reload_type, base::TimeTicks navigation_start, FrameMsg_Navigate_Params* params) { + FrameMsg_UILoadMetricsReportType::Value report_type = + FrameMsg_UILoadMetricsReportType::NO_REPORT; + base::TimeTicks ui_timestamp = base::TimeTicks(); +#if defined(OS_ANDROID) + if (!entry.intent_received_timestamp().is_null()) + report_type = FrameMsg_UILoadMetricsReportType::REPORT_INTENT; + ui_timestamp = entry.intent_received_timestamp(); +#endif + params->common_params = CommonNavigationParams( entry.GetURL(), entry.GetReferrer(), entry.GetTransitionType(), GetNavigationType(controller->GetBrowserContext(), entry, reload_type), - !entry.IsViewSourceMode()); + !entry.IsViewSourceMode(), ui_timestamp, report_type); params->request_params = RequestNavigationParams( entry.GetHasPostData(), entry.extra_headers(), @@ -880,13 +889,24 @@ bool NavigatorImpl::RequestNavigation( int64 frame_tree_node_id = frame_tree_node->frame_tree_node_id(); FrameMsg_Navigate_Type::Value navigation_type = GetNavigationType(controller_->GetBrowserContext(), entry, reload_type); + FrameMsg_UILoadMetricsReportType::Value report_type = + FrameMsg_UILoadMetricsReportType::NO_REPORT; + base::TimeTicks ui_timestamp = base::TimeTicks(); +#if defined(OS_ANDROID) + if (!entry.intent_received_timestamp().is_null()) + report_type = FrameMsg_UILoadMetricsReportType::REPORT_INTENT; + ui_timestamp = entry.intent_received_timestamp(); +#endif + scoped_ptr<NavigationRequest> navigation_request(new NavigationRequest( frame_tree_node, CommonNavigationParams(entry.GetURL(), entry.GetReferrer(), entry.GetTransitionType(), navigation_type, - !entry.IsViewSourceMode()), + !entry.IsViewSourceMode(), + ui_timestamp, + report_type), CommitNavigationParams(entry.GetPageState(), entry.GetIsOverridingUserAgent(), navigation_start))); diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc index 8de801d..00fed25 100644 --- a/content/browser/frame_host/render_frame_host_impl.cc +++ b/content/browser/frame_host/render_frame_host_impl.cc @@ -600,8 +600,15 @@ void RenderFrameHostImpl::OnDocumentOnLoadCompleted( FrameMsg_UILoadMetricsReportType::Value report_type, base::TimeTicks ui_timestamp) { if (report_type == FrameMsg_UILoadMetricsReportType::REPORT_LINK) { - UMA_HISTOGRAM_TIMES("Navigation.UI_OnLoadComplete.Link", - base::TimeTicks::Now() - ui_timestamp); + UMA_HISTOGRAM_CUSTOM_TIMES("Navigation.UI_OnLoadComplete.Link", + base::TimeTicks::Now() - ui_timestamp, + base::TimeDelta::FromMilliseconds(10), + base::TimeDelta::FromMinutes(10), 100); + } else if (report_type == FrameMsg_UILoadMetricsReportType::REPORT_INTENT) { + UMA_HISTOGRAM_CUSTOM_TIMES("Navigation.UI_OnLoadComplete.Intent", + base::TimeTicks::Now() - ui_timestamp, + base::TimeDelta::FromMilliseconds(10), + base::TimeDelta::FromMinutes(10), 100); } // This message is only sent for top-level frames. TODO(avi): when frame tree // mirroring works correctly, add a check here to enforce it. @@ -674,8 +681,18 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) { if (validated_params.report_type == FrameMsg_UILoadMetricsReportType::REPORT_LINK) { - UMA_HISTOGRAM_TIMES("Navigation.UI_OnCommitProvisionalLoad.Link", - base::TimeTicks::Now() - validated_params.ui_timestamp); + UMA_HISTOGRAM_CUSTOM_TIMES( + "Navigation.UI_OnCommitProvisionalLoad.Link", + base::TimeTicks::Now() - validated_params.ui_timestamp, + base::TimeDelta::FromMilliseconds(10), base::TimeDelta::FromMinutes(10), + 100); + } else if (validated_params.report_type == + FrameMsg_UILoadMetricsReportType::REPORT_INTENT) { + UMA_HISTOGRAM_CUSTOM_TIMES( + "Navigation.UI_OnCommitProvisionalLoad.Intent", + base::TimeTicks::Now() - validated_params.ui_timestamp, + base::TimeDelta::FromMilliseconds(10), base::TimeDelta::FromMinutes(10), + 100); } RenderProcessHost* process = GetProcess(); diff --git a/content/common/frame_message_enums.h b/content/common/frame_message_enums.h index 4349a04..a535113 100644 --- a/content/common/frame_message_enums.h +++ b/content/common/frame_message_enums.h @@ -46,7 +46,10 @@ struct FrameMsg_UILoadMetricsReportType { // Report metrics for this load, that originated from clicking on a link. REPORT_LINK, - REPORT_TYPE_LAST = REPORT_LINK, + // Report metrics for this load, that originated from an Android OS intent. + REPORT_INTENT, + + REPORT_TYPE_LAST = REPORT_INTENT, }; }; diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h index 31fa603..9aa03c4 100644 --- a/content/common/frame_messages.h +++ b/content/common/frame_messages.h @@ -210,6 +210,8 @@ IPC_STRUCT_TRAITS_BEGIN(content::CommonNavigationParams) IPC_STRUCT_TRAITS_MEMBER(transition) IPC_STRUCT_TRAITS_MEMBER(navigation_type) IPC_STRUCT_TRAITS_MEMBER(allow_download) + IPC_STRUCT_TRAITS_MEMBER(ui_timestamp) + IPC_STRUCT_TRAITS_MEMBER(report_type) IPC_STRUCT_TRAITS_END() IPC_STRUCT_TRAITS_BEGIN(content::RequestNavigationParams) diff --git a/content/common/navigation_params.cc b/content/common/navigation_params.cc index 31787ac..63e6908 100644 --- a/content/common/navigation_params.cc +++ b/content/common/navigation_params.cc @@ -10,22 +10,28 @@ namespace content { CommonNavigationParams::CommonNavigationParams() : transition(ui::PAGE_TRANSITION_LINK), navigation_type(FrameMsg_Navigate_Type::NORMAL), - allow_download(true) { + allow_download(true), + report_type(FrameMsg_UILoadMetricsReportType::NO_REPORT) { } -CommonNavigationParams::~CommonNavigationParams() {} - CommonNavigationParams::CommonNavigationParams( const GURL& url, const Referrer& referrer, ui::PageTransition transition, FrameMsg_Navigate_Type::Value navigation_type, - bool allow_download) + bool allow_download, + base::TimeTicks ui_timestamp, + FrameMsg_UILoadMetricsReportType::Value report_type) : url(url), referrer(referrer), transition(transition), navigation_type(navigation_type), - allow_download(allow_download) { + allow_download(allow_download), + ui_timestamp(ui_timestamp), + report_type(report_type) { +} + +CommonNavigationParams::~CommonNavigationParams() { } RequestNavigationParams::RequestNavigationParams() : is_post(false) {} diff --git a/content/common/navigation_params.h b/content/common/navigation_params.h index d30d382..b23847c 100644 --- a/content/common/navigation_params.h +++ b/content/common/navigation_params.h @@ -36,7 +36,9 @@ struct CONTENT_EXPORT CommonNavigationParams { const Referrer& referrer, ui::PageTransition transition, FrameMsg_Navigate_Type::Value navigation_type, - bool allow_download); + bool allow_download, + base::TimeTicks ui_timestamp, + FrameMsg_UILoadMetricsReportType::Value report_type); ~CommonNavigationParams(); // The URL to navigate to. @@ -56,6 +58,14 @@ struct CONTENT_EXPORT CommonNavigationParams { // Allows the URL to be downloaded (true by default). // Avoid downloading when in view-source mode. bool allow_download; + + // Timestamp of the user input event that triggered this navigation. Empty if + // the navigation was not triggered by clicking on a link or by receiving an + // intent on Android. + base::TimeTicks ui_timestamp; + + // The report type to be used when recording the metric using |ui_timestamp|. + FrameMsg_UILoadMetricsReportType::Value report_type; }; // Used by FrameMsg_Navigate. diff --git a/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java b/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java index 25a9f763..9724dfe 100644 --- a/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java +++ b/content/public/android/java/src/org/chromium/content_public/browser/LoadUrlParams.java @@ -49,6 +49,7 @@ public class LoadUrlParams { String mVirtualUrlForDataUrl; boolean mCanLoadLocalResources; boolean mIsRendererInitiated; + long mIntentReceivedTimestamp; /** * Creates an instance with default page transition type. @@ -400,6 +401,21 @@ public class LoadUrlParams { return mIsRendererInitiated; } + /** + * @param intentReceivedTimestamp the timestamp at which Chrome received the intent that + * triggered this URL load, as returned by System.currentMillis. + */ + public void setIntentReceivedTimestamp(long intentReceivedTimestamp) { + mIntentReceivedTimestamp = intentReceivedTimestamp; + } + + /** + * @return The timestamp at which Chrome received the intent that triggered this URL load. + */ + public long getIntentReceivedTimestamp() { + return mIntentReceivedTimestamp; + } + public boolean isBaseUrlDataScheme() { // If there's no base url set, but this is a data load then // treat the scheme as data:. diff --git a/content/public/browser/navigation_controller.cc b/content/public/browser/navigation_controller.cc index 6a1e87f..1dbecf2 100644 --- a/content/public/browser/navigation_controller.cc +++ b/content/public/browser/navigation_controller.cc @@ -18,7 +18,10 @@ NavigationController::LoadURLParams::LoadURLParams(const GURL& url) browser_initiated_post_data(NULL), can_load_local_resources(false), should_replace_current_entry(false), - should_clear_history_list(false) { + should_clear_history_list(false) { +#if defined(OS_ANDROID) + intent_received_timestamp = 0; +#endif } NavigationController::LoadURLParams::~LoadURLParams() { @@ -40,6 +43,9 @@ NavigationController::LoadURLParams::LoadURLParams( browser_initiated_post_data(other.browser_initiated_post_data), should_replace_current_entry(false), should_clear_history_list(false) { +#if defined(OS_ANDROID) + intent_received_timestamp = other.intent_received_timestamp; +#endif } NavigationController::LoadURLParams& @@ -60,6 +66,9 @@ NavigationController::LoadURLParams::operator=( browser_initiated_post_data = other.browser_initiated_post_data; should_replace_current_entry = other.should_replace_current_entry; should_clear_history_list = other.should_clear_history_list; +#if defined(OS_ANDROID) + intent_received_timestamp = other.intent_received_timestamp; +#endif return *this; } diff --git a/content/public/browser/navigation_controller.h b/content/public/browser/navigation_controller.h index c8b0c21..ed715036 100644 --- a/content/public/browser/navigation_controller.h +++ b/content/public/browser/navigation_controller.h @@ -174,6 +174,13 @@ class NavigationController { // commits. bool should_clear_history_list; +#if defined(OS_ANDROID) + // On Android, for a load triggered by an intent, the time Chrome received + // the original intent that prompted the load (in milliseconds active time + // since boot). + int64 intent_received_timestamp; +#endif + explicit LoadURLParams(const GURL& url); ~LoadURLParams(); diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc index 7258dbb..d9b5807e3 100644 --- a/content/renderer/render_frame_impl.cc +++ b/content/renderer/render_frame_impl.cc @@ -420,6 +420,17 @@ WebURLRequest CreateURLRequestForNavigation( RequestExtraData* extra_data = new RequestExtraData(); extra_data->set_stream_override(stream_override.Pass()); request.setExtraData(extra_data); + + // Set the ui timestamp for this navigation. Currently the timestamp here is + // only non empty when the navigation was triggered by an Android intent. The + // timestamp is converted to a double version supported by blink. It will be + // passed back to the browser in the DidCommitProvisionalLoad and the + // DocumentLoadComplete IPCs. + base::TimeDelta ui_timestamp = common_params.ui_timestamp - base::TimeTicks(); + request.setUiStartTime(ui_timestamp.InSecondsF()); + request.setInputPerfMetricReportPolicy( + static_cast<WebURLRequest::InputToLoadPerfMetricReportPolicy>( + common_params.report_type)); return request; } @@ -473,6 +484,17 @@ CommonNavigationParams MakeCommonNavigationParams( GURL(request->httpHeaderField(WebString::fromUTF8("Referer")).latin1()), request->referrerPolicy()); params.transition = extra_data->transition_type(); + + // Set the ui timestamp for this navigation. Currently the timestamp here is + // only non empty when the navigation was triggered by an Android intent, or + // by the user clicking on a link. The timestamp is converted from a double + // version supported by blink. It will be passed back to the renderer in the + // CommitNavigation IPC, and then back to the browser again in the + // DidCommitProvisionalLoad and the DocumentLoadComplete IPCs. + params.ui_timestamp = + base::TimeTicks() + base::TimeDelta::FromSecondsD(request->uiStartTime()); + params.report_type = static_cast<FrameMsg_UILoadMetricsReportType::Value>( + request->inputPerfMetricReportPolicy()); return params; } diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 003024b..4a35d9c 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -14675,12 +14675,28 @@ Therefore, the affected-histogram name has to have at least one dot in it. </summary> </histogram> +<histogram name="Navigation.UI_OnCommitProvisionalLoad.Intent" + units="milliseconds"> + <owner>clamy@chromium.org</owner> + <summary> + Time between receiving an Android Intent and the navigation commit. + </summary> +</histogram> + <histogram name="Navigation.UI_OnCommitProvisionalLoad.Link" units="milliseconds"> <owner>clamy@chromium.org</owner> <summary>Time between clicking on a link and the navigation commit.</summary> </histogram> +<histogram name="Navigation.UI_OnLoadComplete.Intent" units="milliseconds"> + <owner>clamy@chromium.org</owner> + <summary> + Time between receiving an Android intent and the document load complete + event for a navigation in the main frame. + </summary> +</histogram> + <histogram name="Navigation.UI_OnLoadComplete.Link" units="milliseconds"> <owner>clamy@chromium.org</owner> <summary> |