RPP: Reland hiding irrelevant buttons in standalone usages

With a revised signal for `canRecord` we now hide the RPP Record button,
and others, in standalone scenarios.

Bug:432043754
Change-Id: I7d470474ec0e481a2e257f2b40519e47b2b999bf
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6943527
Reviewed-by: Connor Clark <cjamcl@chromium.org>
Commit-Queue: Paul Irish <paulirish@chromium.org>

Author: paulirish

front_end/core/host/InspectorFrontendHost.ts

@@ -50,7 +50,7 @@ const MAX_RECORDED_HISTOGRAMS_SIZE = 100;
 const OVERRIDES_FILE_SYSTEM_PATH = '/overrides' as Platform.DevToolsPath.RawPathString;
 
 /**
- * The InspectorFrontendHostStub is a stub interface used the frontend is loaded like a webpage. Examples:
+ * The `InspectorFrontendHostStub` is a stub interface used the frontend is loaded like a webpage. Examples:
  *   - devtools://devtools/bundled/devtools_app.html
  *   - https://chrome-devtools-frontend.appspot.com/serve_rev/@030cc140435b0152645522b9864b75cac6c0a854/worker_app.html
  *   - http://localhost:9222/devtools/inspector.html?ws=localhost:9222/devtools/page/xTARGET_IDx
@@ -484,18 +484,24 @@ export class InspectorFrontendHostStub implements InspectorFrontendHostAPI {
   }
 
   /**
-   * **Hosted mode** is when DevTools is loaded over `http(s)://` rather than from `devtools://`.
-   * It does **not** indicate whether the frontend is connected to a valid CDP target.
+   * Think of **Hosted mode** as "non-embedded" mode; you can see a devtools frontend URL as the tab's URL. It's an atypical way that DevTools is run.
+   * Whereas in **Non-hosted** (aka "embedded"), DevTools is embedded and fully dockable. It's the common way DevTools is run.
    *
-   *  | Example case                                         | Mode           | Example URL                                                                   |
-   *  | :--------------------------------------------------- | :------------- | :---------------------------------------------------------------------------- |
-   *  | typical devtools: (un)docked w/ native CDP bindings  | **NOT Hosted** | `devtools://devtools/bundled/devtools_app.html?targetType=tab&...`            |
-   *  | tab href is `devtools://…?ws=…`                      | **NOT Hosted** | `devtools://devtools/bundled/devtools_app.html?ws=localhost:9228/...`         |
-   *  | tab href is `devtools://…` but no connection         | **NOT Hosted** | `devtools://devtools/bundled/devtools_app.html`                               |
-   *  | tab href is `https://…?ws=` (connected)              | **Hosted**     | `https://chrome-devtools-frontend.appspot.com/serve_rev/@.../worker_app.html` |
-   *  | tab href is `http://…` but no connection             | **Hosted**     | `http://localhost:9222/devtools/inspector.html?ws=localhost:9222/...`         |
+   * **Hosted mode** == we're using the `InspectorFrontendHostStub`. impl. (@see `InspectorFrontendHostStub` class comment)
+   * Whereas with **non-hosted** mode, native `DevToolsEmbedderMessageDispatcher` is used for CDP and more.
+   *
+   * Relationships to other signals:
+   * - Hosted-ness does not indicate whether the frontend is _connected to a valid CDP target_.
+   * - Being _"dockable"_ (aka `canDock`) is typically aligned but technically orthogonal.
+   * - It's unrelated to the _tab's (main frame's) URL_. Though in non-hosted, the devtools frame origin will always be `devtools://devtools`.
    *
-   * See also `canDock` which has similar semantics.
+   *  | Example case                                         | Mode           | Example devtools                                                                   |
+   *  | :--------------------------------------------------- | :------------- | :---------------------------------------------------------------------------- |
+   *  | tab URL: anything. embedded DevTools w/ native CDP bindings    | **NOT Hosted** | `devtools://devtools/bundled/devtools_app.html?targetType=tab&...`            |
+   *  | tab URL: `devtools://…?ws=…`                | **Hosted**     | `devtools://devtools/bundled/devtools_app.html?ws=localhost:9228/...`         |
+   *  | tab URL: `devtools://…` but no connection   | **Hosted**     | `devtools://devtools/bundled/devtools_app.html`                               |
+   *  | tab URL: `https://…` but no connection      | **Hosted**     | `https://chrome-devtools-frontend.appspot.com/serve_rev/@.../worker_app.html` |
+   *  | tab URL: `http://…?ws=` (connected)         | **Hosted**     | `http://localhost:9222/devtools/inspector.html?ws=localhost:9222/...`         |
    */
   isHostedMode(): boolean {
     return true;

front_end/core/sdk/Connections.ts

@@ -281,8 +281,10 @@ function createMainConnection(onConnectionLost: (message: Platform.UIString.Loca
     const ws = (wsParam ? `ws://${wsParam}` : `wss://${wssParam}`) as Platform.DevToolsPath.UrlString;
     return new WebSocketConnection(ws, onConnectionLost);
   }
-  if (Host.InspectorFrontendHost.InspectorFrontendHostInstance.isHostedMode()) {
-    // Hosted mode (e.g. `http://localhost:9222/devtools/inspector.html`) but no WebSocket URL.
+
+  const notEmbeddedOrWs = Host.InspectorFrontendHost.InspectorFrontendHostInstance.isHostedMode();
+  if (notEmbeddedOrWs) {
+    // eg., hosted mode (e.g. `http://localhost:9222/devtools/inspector.html`) without a WebSocket URL,
     return new StubConnection();
   }
 

front_end/core/sdk/TargetManager.ts

@@ -10,6 +10,8 @@ import {assertNotNullOrUndefined} from '../platform/platform.js';
 import type * as ProtocolClient from '../protocol_client/protocol_client.js';
 import * as Root from '../root/root.js';
 
+import {StubConnection} from './Connections.js';
+import {RehydratingConnection} from './RehydratingConnection.js';
 import {SDKModel} from './SDKModel.js';
 import {Target, Type as TargetType} from './Target.js';
 
@@ -310,6 +312,22 @@ export class TargetManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes
     return this.#browserTarget;
   }
 
+  /**
+   * If this returns true, the target is not connected to a legit CDP server.
+   * However, it's not exhaustive, so some `false` responses may be misleading.
+   *    (eg., tab URL of `devtools://devtools/bundled/devtools_app.html` uses a MainConnection but has no CDP server behind it).
+   */
+  hasFakeConnection(): boolean {
+    // There _may_ be a race condition hiding here on the router/connection creation.
+    // So we play it safe and consider "no connection yet" as "not fake".
+    const connection = this.primaryPageTarget()?.router()?.connection();
+    if (!connection) {
+      return false;
+    }
+    const isFakeConnection = (connection instanceof StubConnection) || (connection instanceof RehydratingConnection);
+    return isFakeConnection;
+  }
+
   async maybeAttachInitialTarget(): Promise<boolean> {
     if (!Boolean(Root.Runtime.Runtime.queryParam('browserConnection'))) {
       return false;

front_end/models/live-metrics/LiveMetrics.ts

@@ -534,7 +534,13 @@ export class LiveMetrics extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
 
     const source = await InjectedScript.get();
 
-    const {identifier} = await this.#target.pageAgent().invoke_addScriptToEvaluateOnNewDocument({
+    // Extra check in case the target was removed while we were initializing.
+    // It's possible to be halfway-through enabling when the target is removed
+    // eg try loading 'devtools://devtools/bundled/devtools_app.html?ws=127.0.0.1:99/blah'
+    if (!this.#target) {
+      return;
+    }
+    const {identifier} = await this.#target?.pageAgent().invoke_addScriptToEvaluateOnNewDocument({
       source,
       worldName: LIVE_METRICS_WORLD_NAME,
       runImmediately: true,

front_end/panels/timeline/TimelinePanel.ts

@@ -1053,13 +1053,26 @@ export class TimelinePanel extends Common.ObjectWrapper.eventMixin<EventTypes, t
   }
 
   /**
-   * Returns false if this was loaded in a standalone context such that recording is
-   * not possible, like an enhanced trace (which opens a new devtools window) or
-   * trace.cafe.
+   * Returns false if DevTools is in a standalone context where tracing/recording are NOT available.
+   *
+   * This includes scenarios like:
+   * - viewing an enhanced trace
+   * - viewing a trace in trace.cafe
+   * - other devtools_app.html scenarios without valid `ws=` param.
+   *   - See also the `isHostedMode` comment in `InspectorFrontendHost.ts`
+   *
+   * Possible signals to find a no-record (NR) context:
+   * - `primaryPageTarget()?.sessionId` is empty in NR, but populated when viewing an enhanced trace.
+   * - `primaryPageTarget.#capabilitiesMask` There's a tracing capability but the advertised capabilities are quite unreliable.
+   * - `primaryPageTarget.targets().length === 1` Mostly correct for NC but its 2 when viewing an enhanced trace.
+   * - `primaryPageTarget.router().connection()` Perhaps StubConnection or RehydratingConnection but MainConnection is incorrectly used sometimes. (eg devtools://devtools/bundled/devtools_app.html)
+   * - `resourceTreeModel?.mainFrame === null`. Correct for NR, HOWEVER  Node.js canRecord despite no main frame.
+   * - `rootTarget.type !== 'tab'` Has potential but it lies. (It's "browser" for Node despite a node type)
+   *
+   * The best signal, for now, is this combo (`isNode || hasMainFrame`), which is both well-maintained and correct in all known cases:
    */
   private canRecord(): boolean {
-    // TODO(paulirish) Determine a more robust method as checking `primaryPageTarget()?.sessionId` isn't accurate.
-    return true;
+    return SDK.TargetManager.TargetManager.instance().hasFakeConnection() === false;
   }
 
   private populateToolbar(): void {

front_end/panels/timeline/TimingsTrackAppender.ts

@@ -83,6 +83,7 @@ export class TimingsTrackAppender implements TrackAppender {
         timestampEvents.length === 0 && consoleTimings.length === 0) {
       return trackStartLevel;
     }
+    // TODO(paulirish): these 5 sets of events should be merged and sorted by start time. This would allow for a denser packing.
     this.#appendTrackHeaderAtLevel(trackStartLevel, expanded);
     let newLevel = this.#appendExtensionsAtLevel(trackStartLevel);
     newLevel = this.#compatibilityBuilder.appendEventsAtLevel(performanceMarks, newLevel, this);

front_end/panels/timeline/components/LiveMetricsView.ts

@@ -385,7 +385,9 @@ export class LiveMetricsView extends LegacyWrapper.LegacyWrapper.WrappableCompon
   }
 
   async #refreshFieldDataForCurrentPage(): Promise<void> {
-    await this.#cruxManager.refresh();
+    if (!this.#isNode) {
+      await this.#cruxManager.refresh();
+    }
     void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#render);
   }
 

front_end/ui/visual_logging/KnownContextValues.ts

@@ -2637,6 +2637,7 @@ export const knownContextValues = new Set([
   'node',
   'node-connection',
   'node-id',
+  'node-js-debugging',
   'node-removed',
   'nodes',
   'nominal',