-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI customisations for 0.73 launch #11
Conversation
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 <jacktfranklin@chromium.org> Commit-Queue: Nancy Li <nancyly@chromium.org> (cherry picked from commit cd178ec) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4640224 Auto-Submit: Nancy Li <nancyly@chromium.org> Reviewed-by: Andres Olivares <andoli@chromium.org> Commit-Queue: Andres Olivares <andoli@chromium.org>
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 <nancyly@chromium.org> Commit-Queue: Andres Olivares <andoli@chromium.org> (cherry picked from commit 1f189fb) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4637795 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Auto-Submit: Nancy Li <nancyly@chromium.org> Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Commit-Queue: Nancy Li <nancyly@chromium.org>
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: nancyly@chromium.org Fixed: 1457203 Change-Id: Icf78cbd62bc2b8ed6d9b0b1367832c3a893fe27d Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4637321 Reviewed-by: Danil Somsikov <dsv@chromium.org> Commit-Queue: Nancy Li <nancyly@chromium.org> (cherry picked from commit 9d5feea) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4640222 Commit-Queue: Andres Olivares <andoli@chromium.org> Reviewed-by: Andres Olivares <andoli@chromium.org> Auto-Submit: Nancy Li <nancyly@chromium.org>
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 <dsv@chromium.org> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org> Commit-Queue: Jaroslav Sevcik <jarin@chromium.org> (cherry picked from commit 03d7237) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4654405 Commit-Queue: Danil Somsikov <dsv@chromium.org>
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 <szuend@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> Commit-Queue: Jaroslav Sevcik <jarin@chromium.org> (cherry picked from commit 5fe7b87) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4660618 Commit-Queue: Danil Somsikov <dsv@chromium.org> Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
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 <cjamcl@chromium.org> Commit-Queue: Adam Raine <asraine@chromium.org> Reviewed-by: Connor Clark <cjamcl@chromium.org> (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 <pfaffe@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> (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 <dsv@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org> (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 <dsv@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org> (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 <pfaffe@chromium.org> Commit-Queue: Danil Somsikov <dsv@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> (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 <pfaffe@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> (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 <pfaffe@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> (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 <dsv@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org> Auto-Submit: Philip Pfaffe <pfaffe@chromium.org> Reviewed-by: Danil Somsikov <dsv@chromium.org> (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 <dsv@chromium.org> Reviewed-by: Philip Pfaffe <pfaffe@chromium.org> Commit-Queue: Danil Somsikov <dsv@chromium.org> (cherry picked from commit 21a7422) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4752853 Reviewed-by: Yang Guo <yangguo@chromium.org>
Fixed: 1467751 Change-Id: Iee04d8fa9dfd84ac4ed4b8f4ffb6334fff922c71 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4735433 Reviewed-by: Danil Somsikov <dsv@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org> (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 <dsv@chromium.org> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org> (cherry picked from commit 1b4c7f8) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4757141 Reviewed-by: Yang Guo <yangguo@chromium.org> Reviewed-by: Eric Leese <leese@chromium.org>
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 <paulirish@chromium.org> Commit-Queue: Paul Irish <paulirish@chromium.org> (cherry picked from commit 13bd016) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/4803622 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
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 <motiz88@gmail.com>
// 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'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).
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}); | ||
}, | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
${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"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can refine this back to the debugging
tag once we have some blog posts up!
front_end/entrypoint_template.html
Outdated
@@ -16,5 +16,6 @@ | |||
</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"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still super weird to me that this isn't ./Images/favicon.ico
.
@@ -11,6 +11,7 @@ devtools_image_files = [ | |||
"chromeMiddle.avif", | |||
"chromeRight.avif", | |||
"cssoverview_icons_2x.avif", | |||
"favicon.ico", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turns out to be covered by the //front_end/Images:Images-copy-gen-javascript
rule 👍🏻
Replaced with #12 (retargets primary branch). |
Summary
UI customisations for the React Native 0.73 launch.
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