Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2936723002:
Report frequency of single page app navigations to UMA (Closed)
|
Created:
3 years, 6 months ago by Liquan (Max) Gu Modified:
3 years, 5 months ago CC:
chromium-reviews, blink-reviews, Yoav Weiss, kinuko+watch, platform-architecture-syd+reviews-web_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionReport frequency of single page app navigations to UMA
We currently output a trace event for each single page app navigation. This
change is to record that for UMA as well.
BUG=720550
Review-Url: https://codereview.chromium.org/2936723002
Cr-Commit-Position: refs/heads/master@{#484153}
Committed: https://chromium.googlesource.com/chromium/src/+/772ace0703fb420097d1c14b53a5f186d2027771
Patch Set 1 #
Total comments: 2
Patch Set 2 : update histograms.xml #
Total comments: 7
Patch Set 3 : add test; remove types #Patch Set 4 : Update SPA categories #
Total comments: 1
Patch Set 5 : add others category #Patch Set 6 : add DCHECK, remove others as a category #
Total comments: 8
Patch Set 7 : move enum to header #
Total comments: 2
Patch Set 8 : nit #Patch Set 9 : rebase #Patch Set 10 : add helper function #
Total comments: 1
Patch Set 11 : add unreachable #
Total comments: 3
Patch Set 12 : NOTREACHED #
Messages
Total messages: 61 (24 generated)
maxlg@chromium.org changed reviewers: + tdresser@chromium.org
Make sure to update histograms.xml. Any thoughts on a test for this? Could we do something with HistogramTester? https://codereview.chromium.org/2936723002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): https://codereview.chromium.org/2936723002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:46: kFrameLoadTypeCount I think kFrameLoadTypeLast = kFrameLoadTypeReloadBypassingCache might clean this up a bit - I think you'd no longer need to include it in switches.
On 2017/06/12 20:54:19, tdresser wrote: > Make sure to update histograms.xml. > > Any thoughts on a test for this? Could we do something with HistogramTester? > > https://codereview.chromium.org/2936723002/diff/1/third_party/WebKit/Source/c... > File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): > > https://codereview.chromium.org/2936723002/diff/1/third_party/WebKit/Source/c... > third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:46: kFrameLoadTypeCount > I think > kFrameLoadTypeLast = kFrameLoadTypeReloadBypassingCache > might clean this up a bit - I think you'd no longer need to include it in > switches. I am curious where are the test cases that covers this file?
Patchset #2 (id:20001) has been deleted
https://codereview.chromium.org/2936723002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): https://codereview.chromium.org/2936723002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:46: kFrameLoadTypeCount On 2017/06/12 20:54:19, tdresser wrote: > I think > kFrameLoadTypeLast = kFrameLoadTypeReloadBypassingCache > might clean this up a bit - I think you'd no longer need to include it in > switches. Yes it does. Thanks!
I think tests are here: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/web/tests/WebF... https://codereview.chromium.org/2936723002/diff/40001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2936723002/diff/40001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:60392: + units="ms"> Take a look at another histogram with an enum value. There shouldn't be any units, and you'll need to defined an enum, which lines up with the FrameLoadType enum. It's annoying that there's no good way of testing these changes, other than landing them, and then looking at the dashboard. Thankfully, the dashboard pulls in updates out of band from Chrome updates, so if something is broken, it's generally pretty quick to fix.
japhet@chromium.org changed reviewers: + japhet@chromium.org
https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/FrameLoader.cpp:490: FrameLoadType::kFrameLoadTypeCount); FrameLoadTypes might be overloaded. For example, FrameLoadTypeStandard could be a fragment navigation that adds a history entry, or it could result from a history.pushState() JS call. Do you want to split those down further? Or more generally: What are you trying to accomplish with this data? It's not clear to me what you want to learn from this histogram. https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:36: // existing enums must never be renumbered or deleted and reused. Please don't use this enum directly for UMAs. It's very much an implementation detail of blink, and I'd appreciate the flexibility to reuse or merge values without worrying about numerical consistency.
https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/FrameLoader.cpp:490: FrameLoadType::kFrameLoadTypeCount); On 2017/06/14 18:10:50, Nate Chapin wrote: > FrameLoadTypes might be overloaded. For example, FrameLoadTypeStandard could be > a fragment navigation that adds a history entry, or it could result from a > history.pushState() JS call. Do you want to split those down further? > > Or more generally: What are you trying to accomplish with this data? It's not > clear to me what you want to learn from this histogram. Great question. The high level question we want to answer is "approximately what fraction of navigations are within-app navigations." We just want an approximation, so we figured just counting the number of same single page app navigations, and comparing to a separate page load metric would be good enough. Since there was already a convenient enum lying around to figure out where same document navigations were coming from, we figured we might as well split by that enum. We could probably also get away with just recording the total number of navigations with a load type of: - kStandard - kReplaceCurrentItem Does that sound about right? I don't have a ton of context in this space, let us know if we're completely off base here.
The histogram now just records the total count of same-document-navigation. Please take a look.
lgtm
On 2017/06/14 18:30:52, tdresser wrote: > https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): > > https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/loader/FrameLoader.cpp:490: > FrameLoadType::kFrameLoadTypeCount); > On 2017/06/14 18:10:50, Nate Chapin wrote: > > FrameLoadTypes might be overloaded. For example, FrameLoadTypeStandard could > be > > a fragment navigation that adds a history entry, or it could result from a > > history.pushState() JS call. Do you want to split those down further? > > > > Or more generally: What are you trying to accomplish with this data? It's not > > clear to me what you want to learn from this histogram. > > Great question. > > The high level question we want to answer is "approximately what fraction of > navigations are within-app navigations." > > We just want an approximation, so we figured just counting the number of same > single page app navigations, and comparing to a separate page load metric would > be good enough. > > Since there was already a convenient enum lying around to figure out where same > document navigations were coming from, we figured we might as well split by that > enum. > > We could probably also get away with just recording the total number of > navigations with a load type of: > - kStandard > - kReplaceCurrentItem > > Does that sound about right? I don't have a ton of context in this space, let us > know if we're completely off base here. kStandard and kReplaceCurrentItem are not necessarily interesting, and are overloaded (namely, it won't tell you whether the navigation was a typical fragment navigation or a history API "navigation"). I would guess, if you do want to add more data to this UMA, the 3 most interesting categories are: * history.pushState and history.replaceState * same-document back/forward * other fragment navigations
The CQ bit was checked by maxlg@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
maxlg@chromium.org changed reviewers: + jwd@chromium.org
+jwd@. Please take a look at histograms.xml. Thanks!
On 2017/06/15 18:52:51, Liquan (Max) Gu wrote: > +jwd@. Please take a look at histograms.xml. Thanks! Can we split this out by the three categories Nate identified above? * history.pushState and history.replaceState * same-document back/forward * other fragment navigations I'm fine ignoring the other cases.
On 2017/06/15 19:34:21, tdresser wrote:
> On 2017/06/15 18:52:51, Liquan (Max) Gu wrote:
> > +jwd@. Please take a look at histograms.xml. Thanks!
>
> Can we split this out by the three categories Nate identified above?
>
> * history.pushState and history.replaceState
> * same-document back/forward
> * other fragment navigations
>
> I'm fine ignoring the other cases.
If I understand it correctly, each category corresponds to the following types?
* history.pushState and history.replaceState
kFrameLoadTypeInitialHistoryLoad, kFrameLoadTypeReplaceCurrentItem
* same-document back/forward
kFrameLoadTypeBackForward
* other fragment navigations
(all other load types)
kFrameLoadTypeStandard, kFrameLoadTypeReload,
kFrameLoadTypeInitialInChildFrame, kFrameLoadTypeReloadBypassingCache,
On 2017/06/15 20:08:56, Liquan (Max) Gu wrote: > On 2017/06/15 19:34:21, tdresser wrote: > > On 2017/06/15 18:52:51, Liquan (Max) Gu wrote: > > > +jwd@. Please take a look at histograms.xml. Thanks! > > > > Can we split this out by the three categories Nate identified above? > > > > * history.pushState and history.replaceState > > * same-document back/forward > > * other fragment navigations > > > > I'm fine ignoring the other cases. > > > If I understand it correctly, each category corresponds to the following types? > * history.pushState and history.replaceState > kFrameLoadTypeInitialHistoryLoad, kFrameLoadTypeReplaceCurrentItem > * same-document back/forward > kFrameLoadTypeBackForward > * other fragment navigations > (all other load types) > kFrameLoadTypeStandard, kFrameLoadTypeReload, > kFrameLoadTypeInitialInChildFrame, kFrameLoadTypeReloadBypassingCache, You'll have better look using data from multiple enums: > * history.pushState and history.replaceState > same_document_navigation_source == kSameDocumentNavigationHistoryApi > * same-document back/forward > kFrameLoadTypeBackForward > * other fragment navigations > same_document_navigation_source == kSameDocumentNavigationHistoryDefault and not kFrameLoadTypeBackForward
Patchset #4 (id:80001) has been deleted
Patchset #5 (id:120001) has been deleted
Patchset #4 (id:100001) has been deleted
https://codereview.chromium.org/2936723002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:501: if (same_document_navigation_source == kSameDocumentNavigationHistoryApi) { Can I assume that these three conditions are exclusive and there is no such situation where (source == kSameDocumentNavigationHistoryApi && type == kFrameLoadTypeBackForward)
On 2017/06/16 17:37:36, Liquan (Max) Gu wrote: > https://codereview.chromium.org/2936723002/diff/140001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): > > https://codereview.chromium.org/2936723002/diff/140001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/loader/FrameLoader.cpp:501: if > (same_document_navigation_source == kSameDocumentNavigationHistoryApi) { > Can I assume that these three conditions are exclusive and there is no such > situation where (source == kSameDocumentNavigationHistoryApi && type == > kFrameLoadTypeBackForward) Correct, that situation doesn't ever happen. Though you might want to DCHECK it just to be sure we don't change that :)
Patchset #6 (id:180001) has been deleted
Acknowledged several comments. Please take a look at the latest changes. Unnecessary category "Others" is removed, and I changed to use switch in case new SameDocumentNavigationSource will be added, so that here will catch all the cases. https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/FrameLoader.cpp:490: FrameLoadType::kFrameLoadTypeCount); On 2017/06/14 18:30:52, tdresser wrote: > On 2017/06/14 18:10:50, Nate Chapin wrote: > > FrameLoadTypes might be overloaded. For example, FrameLoadTypeStandard could > be > > a fragment navigation that adds a history entry, or it could result from a > > history.pushState() JS call. Do you want to split those down further? > > > > Or more generally: What are you trying to accomplish with this data? It's not > > clear to me what you want to learn from this histogram. > > Great question. > > The high level question we want to answer is "approximately what fraction of > navigations are within-app navigations." > > We just want an approximation, so we figured just counting the number of same > single page app navigations, and comparing to a separate page load metric would > be good enough. > > Since there was already a convenient enum lying around to figure out where same > document navigations were coming from, we figured we might as well split by that > enum. > > We could probably also get away with just recording the total number of > navigations with a load type of: > - kStandard > - kReplaceCurrentItem > > Does that sound about right? I don't have a ton of context in this space, let us > know if we're completely off base here. > Done. https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): https://codereview.chromium.org/2936723002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:36: // existing enums must never be renumbered or deleted and reused. On 2017/06/14 18:10:50, Nate Chapin wrote: > Please don't use this enum directly for UMAs. It's very much an implementation > detail of blink, and I'd appreciate the flexibility to reuse or merge values > without worrying about numerical consistency. Done. https://codereview.chromium.org/2936723002/diff/40001/tools/metrics/histogram... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2936723002/diff/40001/tools/metrics/histogram... tools/metrics/histograms/histograms.xml:60392: + units="ms"> On 2017/06/13 15:21:29, tdresser wrote: > Take a look at another histogram with an enum value. There shouldn't be any > units, and you'll need to defined an enum, which lines up with the FrameLoadType > enum. > > It's annoying that there's no good way of testing these changes, other than > landing them, and then looking at the dashboard. Thankfully, the dashboard pulls > in updates out of band from Chrome updates, so if something is broken, it's > generally pretty quick to fix. Done.
I got the impression from tdresser@'s comments in the review that the metrics might change. Is that true, or should I look at the histograms now?
jwd@, we're happy with this definition - PTAL. LGTM https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:500: enum SinglePageAppNavigationType { We'd normally define the enum outside of this method. This would let you refer to the enum values in the test for example, which will make the test a bit clearer. https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right): https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/web/tests/WebFrameTest.cpp:12212: tester.ExpectBucketCount(histogramName, 0, 1); Can you index using the enum values? https://codereview.chromium.org/2936723002/diff/200001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2936723002/diff/200001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:60395: + The count of same-document-navigations divided by whether it is the "divided" implies mathematical division. Perhaps "divided" -> "split"?
LGTM w/nit https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:488: // This enum is used to index different kinds of single-page-application Could we put this all of this logging in a helper? It seems like it's complicated enough it deserves its own function.
lgtm with tdresser's histogram nit.
Updated the CL, with something unclear. https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:488: // This enum is used to index different kinds of single-page-application On 2017/06/21 18:05:24, Nate Chapin wrote: > Could we put this all of this logging in a helper? It seems like it's > complicated enough it deserves its own function. By 'all of this logging' do you mean the big switch and UMA_HISTOGRAM_ENUMERATION()? It looks weird to me to mix the logging logic and categorizing logic in a helper function. Helper function with only categorizing logic seems more normal? Let's say, a single_page_application_helper class with FindSinglePageAppNavigationType()? https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:500: enum SinglePageAppNavigationType { On 2017/06/21 14:45:00, tdresser wrote: > We'd normally define the enum outside of this method. This would let you refer > to the enum values in the test for example, which will make the test a bit > clearer. Good idea. https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right): https://codereview.chromium.org/2936723002/diff/200001/third_party/WebKit/Sou... third_party/WebKit/Source/web/tests/WebFrameTest.cpp:12212: tester.ExpectBucketCount(histogramName, 0, 1); On 2017/06/21 14:45:00, tdresser wrote: > Can you index using the enum values? OK, I will put the enum in frameLoaderTypers.h. https://codereview.chromium.org/2936723002/diff/200001/tools/metrics/histogra... File tools/metrics/histograms/histograms.xml (right): https://codereview.chromium.org/2936723002/diff/200001/tools/metrics/histogra... tools/metrics/histograms/histograms.xml:60395: + The count of same-document-navigations divided by whether it is the On 2017/06/21 14:45:00, tdresser wrote: > "divided" implies mathematical division. > > Perhaps "divided" -> "split"? Done.
Thanks, still LGTM, with additional nit. https://codereview.chromium.org/2936723002/diff/220001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): https://codereview.chromium.org/2936723002/diff/220001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:108: // tools/metrics/histograms/enums. nit: /enums.xml
https://codereview.chromium.org/2936723002/diff/220001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoaderTypes.h (right): https://codereview.chromium.org/2936723002/diff/220001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoaderTypes.h:108: // tools/metrics/histograms/enums. On 2017/06/26 15:12:50, tdresser wrote: > nit: /enums.xml Done.
Patchset #9 (id:260001) has been deleted
The CQ bit was checked by maxlg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org, jwd@chromium.org, tdresser@chromium.org Link to the patchset: https://codereview.chromium.org/2936723002/#ps300001 (title: "add helper function")
The CQ bit was unchecked by maxlg@chromium.org
https://codereview.chromium.org/2936723002/diff/300001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/300001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:199: static SinglePageAppNavigationType CategorizeSinglePageAppNavigation( Helper function is created. Nothing functional is changed.
The CQ bit was checked by maxlg@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
Please advise. Thanks! https://codereview.chromium.org/2936723002/diff/320001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/320001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:219: default: Compiling failed because of CQ build's warning "not all control paths return a value" and "warning treated as error". But I think we shouldn't add default so that once new SameDocumentNavigationSource is added, compiler can warn a case to be missing.
The CQ bit was checked by maxlg@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2936723002/diff/320001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/320001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:219: default: On 2017/07/04 18:37:39, Liquan (Max) Gu wrote: > Compiling failed because of CQ build's warning "not all control paths return a > value" and "warning treated as error". > > But I think we shouldn't add default so that once new > SameDocumentNavigationSource is added, compiler can warn a case to be missing. DCHECK(false) -> NOTREACHED() I don't think you need the default case in the switch, you need a return value. So move the NOTREACHED out of the switch, and then return something after the NOTREACHED().
https://codereview.chromium.org/2936723002/diff/320001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/loader/FrameLoader.cpp (right): https://codereview.chromium.org/2936723002/diff/320001/third_party/WebKit/Sou... third_party/WebKit/Source/core/loader/FrameLoader.cpp:219: default: On 2017/07/04 18:42:39, tdresser wrote: > On 2017/07/04 18:37:39, Liquan (Max) Gu wrote: > > Compiling failed because of CQ build's warning "not all control paths return a > > value" and "warning treated as error". > > > > But I think we shouldn't add default so that once new > > SameDocumentNavigationSource is added, compiler can warn a case to be missing. > > DCHECK(false) -> NOTREACHED() > > I don't think you need the default case in the switch, you need a return value. > > So move the NOTREACHED out of the switch, and then return something after the > NOTREACHED(). Done.
The CQ bit was checked by maxlg@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org, tdresser@chromium.org, jwd@chromium.org Link to the patchset: https://codereview.chromium.org/2936723002/#ps290006 (title: "NOTREACHED")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Still LGTM.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
The CQ bit was checked by maxlg@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 290006, "attempt_start_ts": 1499198052322620,
"parent_rev": "e1a594ff1591f048af789cefe0f4ee4648803a43", "commit_rev":
"772ace0703fb420097d1c14b53a5f186d2027771"}
Message was sent while issue was closed.
Description was changed from ========== Report frequency of single page app navigations to UMA We currently output a trace event for each single page app navigation. This change is to record that for UMA as well. BUG=720550 ========== to ========== Report frequency of single page app navigations to UMA We currently output a trace event for each single page app navigation. This change is to record that for UMA as well. BUG=720550 Review-Url: https://codereview.chromium.org/2936723002 Cr-Commit-Position: refs/heads/master@{#484153} Committed: https://chromium.googlesource.com/chromium/src/+/772ace0703fb420097d1c14b53a5... ==========
Message was sent while issue was closed.
Committed patchset #12 (id:290006) as https://chromium.googlesource.com/chromium/src/+/772ace0703fb420097d1c14b53a5... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
