New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 784826 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Tracing UI is broken and no categories show up

Project Member Reported by kraynov@chromium.org, Nov 14 2017

Issue description

Chromium @ f5d2d957af78d8f9cc025e7301d12409d2f48cad (rev 516265)

What steps will reproduce the problem?
(1) Build Chromium from master (checked at revision 516265)
(2) chrome://tracing and 
(3) No categories show up (see screenshot attached).

lalitm@ could you please take a look? Thank you!
 
Screenshot from 2017-11-14 12:57:07.png
25.6 KB View Download

Comment 1 Deleted

revision 516265 - bad
revision 515865 - bad
revision 515465 - good

Comment 3 by lalitm@chromium.org, Nov 14 2017

Status: Started (was: Assigned)
Taking a look at this now.

Comment 4 by lalitm@chromium.org, Nov 14 2017

Cc: benjhayden@chromium.org
So I believe I isolated the change:

commit 3567e9fc0b94f709f01a37dc8a5c0d5b3ba73414
Author: benshayden <benjhayden@chromium.org>
Date:   Thu Nov 2 10:13:08 2017 -0700

    Syntax updates for Polymer 2.0.
    
    This CL is moved from a pull request:
    https://github.com/catapult-project/catapult/pull/4016
    
    Polymer 2.0 is production ready, and we advise all projects to begin migration.
    To aid in that migration, we're writing and applying some automated fixes for
    the mechanical parts of the upgrade.
    
    The <content> element is deprecated, part of the Shadow DOM v0 API which has
    been supplanted by Shadow DOM v1's <slot> element. For very simple uses – like
    those in this PR – this change should be safe and will not require changes to
    the usage sites of elements. Therefore, these changes are backwards-compatible
    with Polymer 1.0 applications.
    
    As <content> is the more powerful API, Polymer 1.0 supports both <content> and
    <slot> by converting <slot> back into <content> at runtime. Polymer 2.0 only
    supports <slot>. There will likely never be cross-browser native support for
    <content>, while Safari and Chrome already support <slot> natively.
    
    This change also updates usage of `@apply` to remove the parentheses. Like
    the other changes in this CL, these are backwards compatible with Polymer 1.0,
    and necessary for Polymer 2.0.
    
    Screenshot of the trace viewer with some events selected, the metrics side
    panel open, and its Export dropdown open:
    https://imgur.com/arvWmUx.png
    
    Change-Id: Iba230c9af872d02eaabb58279da06492e8d58794
    Reviewed-on: https://chromium-review.googlesource.com/751690
    Commit-Queue: Ben Hayden <benjhayden@chromium.org>
    Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>

+benjhayden: any ideas on why this might cause such a breakage?
Cc: -benjhayden@chromium.org
Owner: benjhayden@chromium.org
Looking.
Cc: rictic@google.com
When I try to inspect the <slot> in devtools, the tab crashes in HTMLSlotElement at this DCHECK:
https://codesearch.chromium.org/chromium/src/third_party/WebKit/Source/core/html/HTMLSlotElement.h?q=HTMLSlotElement::FirstDistributedNode&sq=package:chromium&l=60

That DCHECK suggests that trace viewer is combining shadow roots and the slot element incorrectly. Many of the modules in trace viewer are not actually polymer dom-modules. Modules such as Overlay and RecordSelectionDialog are actually subclasses of HTMLDivElement created by tr.ui.b.define(). Overlay calls createShadowRoot directly. Upgrading Overlay to a Polymer dom-module fixes the DCHECK, but breaks RecordSelectionDialog, which can no longer subclass Overlay since Overlay is no longer a subclass of HTMLDivElement. I can upgrade RecordSelectionDialog, but not cleanly or quickly. Also, there are several other tr.ui.b.define()d classes that could be broken. I started upgrading Overlay and RecordSelectionDialog in this CL: https://chromium-review.googlesource.com/#/c/catapult/+/769305

I tested most of the <slot> elements in either M61 or 62 stable on linux, but I did not test Overlay specifically. Trace viewer has both automated tests and over 300 "instantiate" tests, which require interaction and expertise to interpret. The automated tests passed, but overlay_test is clearly broken in our local dev server. I should have examined the relevant instantiation tests more diligently, and if somebody rewrites trace viewer, maybe they can figure out how to automate the instantiate tests.

I think I need to revert 751690 until we can upgrade at least these 2 classes, and possibly others, to Polymer dom-modules.

Peter Burns (rictic): 2 questions:
1. Is there a deadline for updating from <content> to <slot>?
2. Would you be willing to help us upgrade our tr.ui.b.define()s to Polymer dom-modules?

Cc: benjhayden@chromium.org msrchandra@chromium.org ranjitkan@chromium.org rbasuvula@chromium.org nyerramilli@chromium.org
 Issue 784786  has been merged into this issue.

Comment 9 Deleted

Status: Fixed (was: Started)
The revert has landed, so I believe this should be fixed.

As a followup, I'm re-landing the safe parts of 751690 in https://chromium-review.googlesource.com/769649
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 1

The following revision refers to this bug:
  https://chromium.googlesource.com/catapult/+/16717a6dde421e861077b42fc69db40f1764ca5a

commit 16717a6dde421e861077b42fc69db40f1764ca5a
Author: Ben Hayden <benjhayden@chromium.org>
Date: Tue Jan 01 20:27:53 2019

Reland the safe parts of "Syntax updates for Polymer 2.0."

Polymer 2.0 is production ready, and we advise all projects to begin
migration. To aid in that migration, we're writing and applying some
automated fixes for the mechanical parts of the upgrade.

The <content> element is deprecated, part of the Shadow DOM v0 API which
has been supplanted by Shadow DOM v1's <slot> element. For very simple
uses – like those in this PR – this change should be safe and will not
require changes to the usage sites of your element. Therefor, these
changes are backwards-compatible with Polymer 1.0 applications.

As <content> is the more powerful API, Polymer 1.0 supports both
<content> and <slot> by converting <slot> back into <content> at
runtime. Polymer 2.0 only supports <slot>. There will likely never be
cross-browser native support for <content>, while Safari and Chrome
already support <slot> natively.

This change also updates usage of `@apply` to remove the parentheses.
Like the other changes in this CL, these are backwards compatible with
Polymer 1.0, and necessary for Polymer 2.0.

Original pull request: https://github.com/catapult-project/catapult/pull/4016
Original gerrit CL: https://chromium-review.googlesource.com/751690
Revert: https://chromium-review.googlesource.com/769447

This reland is a more direct copy of the original pull request, 4016.
The original gerrit CL, 751690, incorrectly migrated some content
elements that are not contained in dom-modules, such as the content
element in overlay.html.
Using slot elements in v0 shadow roots (such as created by
createShadowRoot in overlay.html), outside of Polymer dom-modules is not
supported.
This CL keeps the content element in overlay.html.
When Overlay and RecordSelectionDialog are upgraded to Polymer
dom-modules, their content elements should be changed to slot elements
at that time.

We can keep using ::content CSS rules with slot elements in Polymer 1.0
because Polymer 1.0 converts the <slot> elements to <content> at runtime.
However, Polymer 2.0 does not, so when trace viewer migrates to Polymer 2.0,
the ::content CSS rules will need to change to `::slotted`. However, there
are some caveats. ::slotted isn't as powerful as ::content, so we may need
to redesign some elements to not rely on ::slotted.

Bug:  chromium:784826 
Change-Id: I0fb3ca680c83c53297411d27fbfc4aa4fc2e2961
Reviewed-on: https://chromium-review.googlesource.com/c/769649
Commit-Queue: Ben Hayden <benjhayden@chromium.org>
Reviewed-by: Ethan Kuefner <eakuefner@chromium.org>

[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/base/dropdown.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/timeline_view.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/base/toolbar_button.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/base/tab_view.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/timeline_track_view.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/analysis/single_object_snapshot_sub_view.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/analysis/analysis_view.html
[modify] https://crrev.com/16717a6dde421e861077b42fc69db40f1764ca5a/tracing/tracing/ui/analysis/analysis_link.html

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 1

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8572cea4777e82e55ed8a89cd20009540ce83985

commit 8572cea4777e82e55ed8a89cd20009540ce83985
Author: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Date: Tue Jan 01 22:20:43 2019

Roll src/third_party/catapult ecf56e8e03cd..16717a6dde42 (1 commits)

https://chromium.googlesource.com/catapult.git/+log/ecf56e8e03cd..16717a6dde42


git log ecf56e8e03cd..16717a6dde42 --date=short --no-merges --format='%ad %ae %s'
2019-01-01 benjhayden@chromium.org Reland the safe parts of "Syntax updates for Polymer 2.0."


Created with:
  gclient setdep -r src/third_party/catapult@16717a6dde42

The AutoRoll server is located here: https://autoroll.skia.org/r/catapult-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff, who should
be CC'd on the roll, and stop the roller if necessary.

CQ_INCLUDE_TRYBOTS=luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel

BUG= chromium:784826 
TBR=sullivan@chromium.org

Change-Id: I20f90e5d9b4ea9661d6ef54d954ed2ecb6f0414c
Reviewed-on: https://chromium-review.googlesource.com/c/1393084
Reviewed-by: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Commit-Queue: chromium-autoroll <chromium-autoroll@skia-public.iam.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#619358}
[modify] https://crrev.com/8572cea4777e82e55ed8a89cd20009540ce83985/DEPS

Sign in to add a comment