UI customisations for 0.73 launch #11

huntie · 2023-09-21T15:04:57Z

Summary

UI customisations for the React Native 0.73 launch.

Disables the Network and Performance panels.
Enables the legacy JS Profiler panel — currently compatible with Hermes in 0.73.
Adds a favicon and updates the window title to "DevTools (React Native)".
Adds a "Technology Preview" label to the Welcome panel.
- Note: In 0.74, we plan to do a harder launch along with the improved CDP lifecycle + backend.
Fixes link targets in Welcome panel.

Note

Please rebase and merge to preserve individual commits.

Test plan

Favicon and window title

Updated Welcome screen

Disabled Network, Performance panels — and enabled legacy Profiler panel

The .cpuprofile file format was still a trace (with traceEvents) but it should match the CDP Profiler.Profile type. Bug: 1456799 Change-Id: I36af40cb9b66b98163f47768adc4284ecde34595 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4632722 Reviewed-by: Jack Franklin <[email protected]> Commit-Queue: Nancy Li <[email protected]> (cherry picked from commit cd178ec) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4640224 Auto-Submit: Nancy Li <[email protected]> Reviewed-by: Andres Olivares <[email protected]> Commit-Queue: Andres Olivares <[email protected]>

Before this CL the event listener that loads a profile via console.profile() was added using TargetManager::addModelListener. This is problematic because there is no guarantee that the model exists by the time the event listener is added (f.e. if the target hasn't been created). As a consequence, using the console.profile() API wouldn't work under this condition: Open Node DevTools with the Performance panel as the initially selected tab. This CL fixes this race condition by adding the event listnerer to the model only when the model has been created. Fixed: 1457197 Change-Id: I3143d02c197b7b9be17b2b5ba8596ab20c65d148 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4637161 Reviewed-by: Nancy Li <[email protected]> Commit-Queue: Andres Olivares <[email protected]> (cherry picked from commit 1f189fb) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4637795 Bot-Commit: Rubber Stamper <[email protected]> Auto-Submit: Nancy Li <[email protected]> Commit-Queue: Rubber Stamper <[email protected]> Commit-Queue: Nancy Li <[email protected]>

We realized we were getting console errors when running DevTools in node debugging mode (node_app). The errors where about SDK models used by the outermost target selector feature not being available, which is expected because the ispected target is node. To resolve this, we moved the feature registration from main to inspector_main, which isn't part of the node_app. Co-authored-by: [email protected] Fixed: 1457203 Change-Id: Icf78cbd62bc2b8ed6d9b0b1367832c3a893fe27d Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4637321 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Nancy Li <[email protected]> (cherry picked from commit 9d5feea) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4640222 Commit-Queue: Andres Olivares <[email protected]> Reviewed-by: Andres Olivares <[email protected]> Auto-Submit: Nancy Li <[email protected]>

fixes an issue with the network tab where components would render multiple times per frame, causing slowness/lag. This fixes that issue by passing a label to RenderCoordinator.write so that the network components only update once per frame. How to test: Check out the steps at https://bugs.chromium.org/p/chromium/issues/detail?id=1447912#c3 Bug: 1447912 Change-Id: I58ab4af5b89fed93f77caabf4cda66e56b9ee833 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4642732 Reviewed-by: Danil Somsikov <[email protected]> Reviewed-by: Jaroslav Sevcik <[email protected]> Commit-Queue: Jaroslav Sevcik <[email protected]> (cherry picked from commit 03d7237) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4654405 Commit-Queue: Danil Somsikov <[email protected]>

This makes sure that the breakpoint controller provides the initial breakpoint data for BreakpointView. Bug: 1458738 Change-Id: I2dca60f4757e52831d32472683ff9b76d7eb7e61 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4650555 Reviewed-by: Simon Zünd <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Jaroslav Sevcik <[email protected]> (cherry picked from commit 5fe7b87) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4660618 Commit-Queue: Danil Somsikov <[email protected]> Reviewed-by: Jaroslav Sevcik <[email protected]>

The affected code is minified and difficult to review, but the only change made was switching the position of some elements to `fixed`: GoogleChrome/lighthouse#15181 It is still unkown why `absolute` positioning caused this bug, but this CL should fix the symptoms until the root cause is found. Bug: 1439785 Change-Id: I04caee5e78d4c8257e4e4152e995f2c448fd0c34 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4633481 Commit-Queue: Connor Clark <[email protected]> Commit-Queue: Adam Raine <[email protected]> Reviewed-by: Connor Clark <[email protected]> (cherry picked from commit e4c1157) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4681007

Fixed: 1454544 Change-Id: I5917e42696111ea7ee83b856f96a9d67fbe08ea3 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4660496 Commit-Queue: Philip Pfaffe <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> (cherry picked from commit 6aed5e3) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4684209

Bug: 1429353 Change-Id: Ic5532f71d9930cdef797d3bec41066c0180ff7fb Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4633560 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]> (cherry picked from commit 8e7f800) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4685548

Bug: 1429353 Change-Id: I2605e77f74f08e0e3619b88440641838adf34679 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4637291 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]> (cherry picked from commit 2d19bcb) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4684351

Bug: 1451146 Change-Id: I59219bfa59c090264c7a074507520002b9d93384 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4667235 Auto-Submit: Philip Pfaffe <[email protected]> Commit-Queue: Danil Somsikov <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> (cherry picked from commit b94a5dd) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4684218

- Eliminate second path to add extensions for tests: We can use the regular extension registration path in tests except for the iframe. Pull the iframe setup into a separate method that tests can stub away. - Correctly reset the devtools API object between tests. With the switch to new headless we were always hitting the catch in cleanup(). Bug: 1451146 Change-Id: I2d23f15690d9e04afac8df9190e222cb442dc5a4 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4664805 Commit-Queue: Philip Pfaffe <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> (cherry picked from commit a88e571) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4685557

When the target isn't fully initialized or its inspected url isn't set yet extension registration would fail-open. With this CL we defer loading extensions until there is an inspected url. Bug: 1451146, 1461895 Change-Id: Iac7a3323f561f538706c59b8e10c75ce0e3364b6 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4664806 Commit-Queue: Philip Pfaffe <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> (cherry picked from commit aa7dddc) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4679109

Fixed: 1461895 Change-Id: Ic8b35ca87082c6ccbce6a7ab6937af03e31354b1 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4675376 Commit-Queue: Danil Somsikov <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]> Auto-Submit: Philip Pfaffe <[email protected]> Reviewed-by: Danil Somsikov <[email protected]> (cherry picked from commit 475e75f) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4685571

Bug: 1467169 Change-Id: Ic663af79bc9a2b6b60368e59515735f6cba57a1c Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4714725 Auto-Submit: Danil Somsikov <[email protected]> Reviewed-by: Philip Pfaffe <[email protected]> Commit-Queue: Danil Somsikov <[email protected]> (cherry picked from commit 21a7422) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4752853 Reviewed-by: Yang Guo <[email protected]>

Fixed: 1467751 Change-Id: Iee04d8fa9dfd84ac4ed4b8f4ffb6334fff922c71 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4735433 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]> (cherry picked from commit 27078e3) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4748502

When accessing the contents of page resources or network requests, check against extension permissions. It's still possible to enumerate resources or requests including those for unpermitted URLs, but accessing their content is prevented. Fixed: 1467743 Change-Id: I36d0ed38da4429eb1d064f3483efdee9d5565482 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4738687 Reviewed-by: Danil Somsikov <[email protected]> Commit-Queue: Philip Pfaffe <[email protected]> (cherry picked from commit 1b4c7f8) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4757141 Reviewed-by: Yang Guo <[email protected]> Reviewed-by: Eric Leese <[email protected]>

Lighthouse bug: GoogleChrome/lighthouse#15300 Bug: 1472050 Change-Id: I218372aa27699b2e744813a3ab228f9db3464c2a Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4762984 Reviewed-by: Paul Irish <[email protected]> Commit-Queue: Paul Irish <[email protected]> (cherry picked from commit 13bd016) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4803622 Bot-Commit: Rubber Stamper <[email protected]>

This helps with local development through a symlink / submodule.

* Create Welcome panel in rn_inspector * Update RN debugger brand name to "React Native JS Inspector" --------- Co-authored-by: Moti Zilberman <[email protected]>

config/gni/devtools_grd_files.gni

front_end/entrypoints/rn_inspector/rn_inspector.ts

huntie · 2023-09-22T16:42:44Z

front_end/entrypoints/main/MainImpl.ts

-    // JS Profiler
-    Root.Runtime.experiments.register(
-        'jsProfilerTemporarilyEnable', 'Enable JavaScript Profiler temporarily', /* unstable= */ false,
-        'https://developer.chrome.com/blog/js-profiler-deprecation/',
-        'https://bugs.chromium.org/p/chromium/issues/detail?id=1354548');
-


This is removed, and replicated inside rn_inspector.ts. This is to register the experiment in time to enable it in that context — otherwise we're unable to cleanly wait for MainImpl to initialise (without making other changes).

huntie · 2023-09-22T16:44:32Z

front_end/panels/js_profiler/js_profiler-meta.ts

-  persistence: UI.ViewManager.ViewPersistence.CLOSEABLE,
+  persistence: UI.ViewManager.ViewPersistence.PERMANENT,
  experiment: Root.Runtime.ExperimentName.JS_PROFILER_TEMP_ENABLE,
  async loadView() {
    const Profiler = await loadProfilerModule();
    return Profiler.ProfilesPanel.JSProfilerPanel.instance();
  },
 });

-UI.ViewManager.registerViewExtension({
-  location: UI.ViewManager.ViewLocationValues.PANEL,
-  id: 'timeline',
-  title: i18nLazyString(UIStrings.performance),
-  commandPrompt: i18nLazyString(UIStrings.showPerformance),
-  order: 66,
-  hasToolbar: false,
-  isPreviewFeature: true,
-  async loadView() {
-    const Timeline = await loadTimelineModule();
-    return Timeline.TimelinePanel.TimelinePanel.instance({forceNew: null, isNode: true});
-  },
-});
-


The js_profiler.js entry point comes with two panels, per the pre-removal state in Chrome DevTools. Direct tweaks are made here — relatively isolated given the other DevTools entry points refer to the modern timeline-meta.ts instead.

huntie · 2023-09-22T16:45:41Z

front_end/panels/rn_welcome/RNWelcome.ts

            ${i18nString(UIStrings.docsLabel)}
          </x-link>
-          <x-link class="devtools-link" href="https://reactnative.dev/blog/tags/debugging">
+          <x-link class="devtools-link" href="https://reactnative.dev/blog">


We can refine this back to the debugging tag once we have some blog posts up!

motiz88 · 2023-09-25T21:26:41Z

front_end/entrypoint_template.html

 </style>
 <meta http-equiv="Content-Security-Policy" content="object-src 'none'; script-src 'self' 'unsafe-eval' https://chrome-devtools-frontend.appspot.com">
 <meta name="referrer" content="no-referrer">
+<link rel="icon" href="/Images/favicon.ico">


Still super weird to me that this isn't ./Images/favicon.ico.

motiz88 · 2023-09-25T21:26:48Z

config/gni/devtools_image_files.gni

  "chromeMiddle.avif",
  "chromeRight.avif",
  "cssoverview_icons_2x.avif",
+  "favicon.ico",


AFAICT the images listed here are (1) meant to be referenced by JS, (2) passed through an image optimisation pipeline that I'm not sure is compatible with .ico files. We would probably want to define a separate build action somewhere that just copies favicon.ico to somewhere under front_end/.

Turns out to be covered by the //front_end/Images:Images-copy-gen-javascript rule 👍🏻

huntie · 2023-09-26T15:39:58Z

Replaced with #12 (retargets primary branch).

Nancy Li and others added 30 commits June 23, 2023 15:23

Fix Node hooks on Apple Silicon

c888824

Add gclient generated files to gitignore

5f52063

This helps with local development through a symlink / submodule.

Scaffold new entry point for RN inspector

dd51bf9

Hide filesystem/workspaces UI if not available

8fa79ca

Hide most unnecessary UI in rn_inspector

cf2cd55

Add README for rn_inspector

53ab951

Setup CI for fork

ef43cd5

Create Welcome panel in rn_inspector (#8)

a13c62e

* Create Welcome panel in rn_inspector * Update RN debugger brand name to "React Native JS Inspector" --------- Co-authored-by: Moti Zilberman <[email protected]>

Update LICENSE and license headers in modified directories (#9)

089861c

Create simple index.html file for inspector

b6e270a

Remote Network panel from rn_inspector (#10)

da1494e

Mark non-entrypoint RNWelcome files as debug-only

ce61551

Relativize RN welcome icon URL

0ffb687

motiz88 requested changes Sep 22, 2023

View reviewed changes

config/gni/devtools_grd_files.gni Show resolved Hide resolved

front_end/entrypoints/rn_inspector/rn_inspector.ts Show resolved Hide resolved

huntie commented Sep 22, 2023

View reviewed changes

motiz88 reviewed Sep 25, 2023

View reviewed changes

huntie added 5 commits September 26, 2023 11:37

Customise window title and favicon

a6c73da

Add "Technology Preview" label to Welcome panel

1c92ad9

Update Welcome panel links

df72105

Disable Performance panel, enable legacy JS Profiler

a47122b

Disable Network panel

cbd54e2

huntie mentioned this pull request Sep 26, 2023

UI customisations for 0.73 launch #12

Merged

huntie closed this Sep 26, 2023

huntie deleted the 0.73-launch branch September 27, 2023 14:38

motiz88 pushed a commit that referenced this pull request Mar 19, 2024

Tweak experiment flag check within legacy JS Profiler panel (#11)

9370b35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

UI customisations for 0.73 launch #11

UI customisations for 0.73 launch #11

Uh oh!

huntie commented Sep 21, 2023 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

huntie Sep 22, 2023 •

edited

Loading

Uh oh!

huntie Sep 22, 2023 •

edited

Loading

Uh oh!

huntie Sep 22, 2023

Uh oh!

motiz88 Sep 25, 2023

Uh oh!

motiz88 Sep 25, 2023

Uh oh!

huntie Sep 26, 2023

Uh oh!

huntie commented Sep 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

UI customisations for 0.73 launch #11

UI customisations for 0.73 launch #11

Uh oh!

Conversation

huntie commented Sep 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

Uh oh!

Uh oh!

huntie Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huntie Sep 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huntie Sep 22, 2023

Choose a reason for hiding this comment

Uh oh!

motiz88 Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

motiz88 Sep 25, 2023

Choose a reason for hiding this comment

Uh oh!

huntie Sep 26, 2023

Choose a reason for hiding this comment

Uh oh!

huntie commented Sep 26, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

huntie commented Sep 21, 2023 •

edited

Loading

huntie Sep 22, 2023 •

edited

Loading

huntie Sep 22, 2023 •

edited

Loading