Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(304)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: tracing/tracing/ui/side_panel/frame_data_side_panel.html

Issue 1996303002: [Tracing] Introduce Frame Data Side Panel (Closed) Base URL: https://github.com/catapult-project/catapult.git@master
Patch Set: Respond to ROI; Merge Frame and FTN Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« tracing/tracing/model/model.html ('K') | « tracing/tracing/ui/extras/lean_config.html ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: tracing/tracing/ui/side_panel/frame_data_side_panel.html
diff --git a/tracing/tracing/ui/side_panel/frame_data_side_panel.html b/tracing/tracing/ui/side_panel/frame_data_side_panel.html
new file mode 100644
index 0000000000000000000000000000000000000000..36e072abaddd76eb9134f1515412f66d6c7c0260
--- /dev/null
+++ b/tracing/tracing/ui/side_panel/frame_data_side_panel.html
@@ -0,0 +1,229 @@
+<!DOCTYPE html>
+<!--
+Copyright 2015 The Chromium Authors. All rights reserved.
petrcermak 2016/05/24 10:54:11 2016
Xiaocheng 2016/05/25 10:17:48 Done.
+Use of this source code is governed by a BSD-style license that can be
+found in the LICENSE file.
+-->
+
+<link rel="import" href="/tracing/base/statistics.html">
petrcermak 2016/05/24 10:54:11 you shouldn't need this
Xiaocheng 2016/05/25 10:17:47 Done. I just copied this part from file_size_stats
+<link rel="import" href="/tracing/ui/base/grouping_table.html">
petrcermak 2016/05/24 10:54:11 I don't think you're using grouping table. You sho
+<link rel="import" href="/tracing/ui/base/grouping_table_groupby_picker.html">
+<link rel="import" href="/tracing/ui/side_panel/side_panel.html">
+<link rel="import" href="/tracing/value/numeric.html">
petrcermak 2016/05/24 10:54:10 you shouldn't need this
+<link rel="import" href="/tracing/value/ui/scalar_span.html">
+<link rel="import" href="/tracing/value/unit.html">
+
+<polymer-element name='tr-ui-sp-frame-data-side-panel'
+ extends='tr-ui-side-panel'>
+ <template>
+ <style>
+ :host {
+ display: flex;
+ flex-direction: column;
+ }
+ /**
+ * TODO(xiaochengh): It's ugly when the width of the panel is enlarged.
petrcermak 2016/05/24 10:54:10 This is the CSS selector you're probably looking f
Xiaocheng 2016/05/25 10:17:47 Doesn't work :( The real content of tr-ui-b-table
+ * Should fix the width of the URL column instead (how?).
+ */
+ table-container {
+ display: flex;
+ min-height: 0px;
petrcermak 2016/05/24 10:54:11 Do you really need this?
Xiaocheng 2016/05/25 10:17:47 Removed.
+ width: 600px;
+ overflow-y: auto;
+ }
+ </style>
+ <table-container>
+ <tr-ui-b-table id="table"></tr-ui-b-table>
+ </table-container>
+ </template>
+</polymer-element>
+
+<script>
+'use strict';
+(function() {
petrcermak 2016/05/24 10:54:11 Please use tr.exportTo instead. Sample usage: http
Xiaocheng 2016/05/25 10:17:47 The file is now moved to ui/extras/side_panel/ and
+
+ function Row() {
+ this.contexts = [];
+ this.type = undefined;
+ this.renderer = undefined;
+ this.eventSet = undefined;
+ }
+
+ Row.prototype = {
+ get url() {
+ var ans = undefined;
+ this.contexts.some(context => ans = context.args.url);
+ return ans || '';
+ },
+
+ get time() {
+ var ans = 0;
+ this.eventSet.forEach(event => ans += event.selfTime || 0);
+ return ans;
+ },
+
+ calcEventSet: function(rangeOfInterest) {
+ var ans = new tr.model.EventSet();
+ this.contexts.forEach(context => context.eventSet.filter(event =>
petrcermak 2016/05/24 10:54:11 Please make this code more readable by adding extr
Xiaocheng 2016/05/25 10:17:47 This part has been rewritten.
+ rangeOfInterest.intersectsExplicitRangeInclusive(event.start, event.end)
+ ).forEach(event => ans.push(event)));
+ this.eventSet = ans;
+ }
+ };
+
+ Polymer('tr-ui-sp-frame-data-side-panel', {
+ ready: function() {
+ this.model_ = undefined;
+ this.rangeOfInterest_ = new tr.b.Range();
+
+ // TODO(xiaochengh): Design proper grouping of the rows (by renderer pid,
+ // frame tree topology, site, ...).
+ this.$.table.showHeader = true;
+ this.$.table.selectionMode = tr.ui.b.TableFormat.SelectionMode.ROW;
+ this.$.table.tableColumns = this.frameDataTableColumns_();
+ this.$.table.addEventListener('selection-changed', function(e) {
+ var row = this.$.table.selectedTableRow;
petrcermak 2016/05/24 10:54:10 nit: I don't think there's any value in naming thi
Xiaocheng 2016/05/25 10:17:47 Done.
+ this.selectSlicesOfContext_(row);
+ }.bind(this));
+ },
+
+ /**
+ * Fires a selection event selecting all slices of the specified context.
+ */
+ selectSlicesOfContext_: function(row) {
petrcermak 2016/05/24 10:54:10 I think it would make more sense to pass the event
Xiaocheng 2016/05/25 10:17:47 Done.
+ var event = new tr.model.RequestSelectionChangeEvent();
+ event.selection = row.eventSet;
+ this.dispatchEvent(event);
+ },
+
+ frameDataTableColumns_: function() {
+ var columns = [];
petrcermak 2016/05/24 10:54:11 The number of columns is fixed, so please do the f
Xiaocheng 2016/05/25 10:17:47 Done.
+ columns.push({
+ title: 'Renderer',
petrcermak 2016/05/24 10:54:11 nit: this should be indented only 2 spaces (-2)
Xiaocheng 2016/05/25 10:17:47 Done.
+ value: row => row.renderer,
+ cmp: (a, b) => a.renderer - b.renderer
+ });
+ columns.push({
+ title: 'Type',
+ value: row => row.type
+ });
+ // TODO(xiaochengh): Decide what details to show in the table.
+ columns.push({
+ title: 'Time',
+ value: row => tr.v.ui.createScalarSpan(
+ row.time, {
petrcermak 2016/05/24 10:54:11 nit: I would put this on the previous line
Xiaocheng 2016/05/25 10:17:47 Done.
+ unit: tr.v.Unit.byName.timeStampInMs,
+ ownerDocument: this.ownerDocument
+ }
+ ),
+ cmp: (a, b) => a.time - b.time
+ });
+ columns.push({
+ title: 'URL',
+ value: row => row.url,
+ cmp: (a, b) => a.url.localeCompare(b.url)
+ });
+ return columns;
+ },
+
+ frameDataTableRows_: function() {
petrcermak 2016/05/24 10:54:11 The name of a method should start with a verb (e.g
Xiaocheng 2016/05/25 10:17:47 Done.
+ if (!this.model_)
+ return [];
+
+ var rows = [];
+ for (var pid in this.model_.processes) {
+ this.model_.processes[pid].objects.getAllObjectInstances().filter(
petrcermak 2016/05/24 10:54:10 By calling getAllObjectInstances and filter, you a
Xiaocheng 2016/05/25 10:17:47 Done. Thanks for the advice.
+ instance => instance instanceof tr.e.chrome.BlameContextInstance
petrcermak 2016/05/24 10:54:11 You really shouldn't access extras (tr.e.*) from g
Xiaocheng 2016/05/25 10:17:47 Done with polymorphism.
+ ).forEach(objectInstance =>
+ objectInstance.snapshots.forEach(function(snapshot) {
+ // TODO(xiaocheng): Replace type-based branching by polymorphism?
+ if (snapshot instanceof tr.e.chrome.FrameSnapshot)
+ return;
+ var row = new Row();
+ row.contexts.push(snapshot);
+ if (snapshot instanceof tr.e.chrome.FrameTreeNodeSnapshot) {
+ row.type = 'Frame';
+ var renderFrame = snapshot.args.RenderFrame;
+ if (renderFrame instanceof tr.e.chrome.FrameSnapshot) {
+ row.contexts.push(renderFrame);
+ row.renderer = renderFrame.objectInstance.parent.pid;
+ }
+ } else if (snapshot instanceof tr.e.chrome.TopLevelSnapshot) {
+ row.type = 'TopLevel';
+ row.renderer = pid;
+ } else {
+ throw new Error('Unknown context type');
+ }
+ row.calcEventSet(this.currentRangeOfInterest);
+ rows.push(row);
+ }.bind(this)));
+ }
+ return rows;
+ },
+
+ updateContents_: function() {
+ this.$.table.tableRows = this.frameDataTableRows_();
+ this.$.table.rebuild();
+ },
+
+ supportsModel: function(m) {
+ if (!m) {
+ return {
+ supported: false,
+ reason: 'No model available.'
+ };
+ }
+
+ for (var pid in m.processes) {
+ if (m.processes[pid].objects.getAllObjectInstances().some(
petrcermak 2016/05/24 10:54:11 Again, please use iterObjectInstances to avoid was
Xiaocheng 2016/05/25 10:17:47 Done.
+ object => object instanceof tr.e.chrome.BlameContextInstance))
Sami 2016/05/24 09:49:57 nit: add braces around the multi-line if.
Xiaocheng 2016/05/25 10:17:47 No need now.
+ return {
+ supported: true
+ };
+ }
+
+ return {
+ supported: false,
+ reason: 'No frame data available.'
+ };
+ },
+
+ get currentRangeOfInterest() {
+ if (this.rangeOfInterest_.isEmpty)
+ return this.model_.bounds;
+ else
+ return this.rangeOfInterest_;
+ },
+
+ get rangeOfInterest() {
+ return this.rangeOfInterest_;
+ },
+
+ set rangeOfInterest(rangeOfInterest) {
+ this.rangeOfInterest_ = rangeOfInterest;
+ this.updateContents_();
+ },
+
+ get selection() {
+ // Not applicable.
+ },
+
+ set selection(_) {
+ // Not applicable.
+ },
+
+ get textLabel() {
+ return 'Frame Data';
petrcermak 2016/05/24 10:54:11 nit: invalid indentation (should be 2 spaces)
Xiaocheng 2016/05/25 10:17:47 Done.
+ },
+
+ get model() {
+ return this.model_;
+ },
+
+ set model(model) {
+ this.model_ = model;
+ this.updateContents_();
+ }
+ });
+})();
+</script>
« tracing/tracing/model/model.html ('K') | « tracing/tracing/ui/extras/lean_config.html ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698