RPP: Simplify NavigationStart types

The delineation isn't worth the oddities of these types.

Bug:1457310
Change-Id: I349b116491dd5d840618890a4fe598ac9230ceed
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6183083
Commit-Queue: Connor Clark <[email protected]>
Auto-Submit: Paul Irish <[email protected]>
Reviewed-by: Connor Clark <[email protected]>

Author: paulirish

front_end/models/trace/handlers/MetaHandler.test.ts

@@ -50,7 +50,8 @@ describe('MetaHandler', function() {
         tid: Trace.Types.Events.ThreadID(775),
         ts: Trace.Types.Timing.Micro(800),
         name: 'navigationStart',
-      } as Trace.Types.Events.NavigationStartUnreliable,
+        // Casting as NavStart, but it is ignored due to the empty documentLoaderURL
+      } as Trace.Types.Events.NavigationStart,
       {
         ...defaultTraceEvent,
         args: {

front_end/models/trace/insights/Common.test.ts

@@ -17,7 +17,11 @@ describeWithEnvironment('Common', function() {
         throw new Error('missing metadata');
       }
 
-      const insightSetKey = getFirstOrError(data.Meta.navigationsByNavigationId.values()).args.data.navigationId;
+      const firstNav = getFirstOrError(data.Meta.navigationsByNavigationId.values());
+      if (!firstNav.args.data?.navigationId) {
+        throw new Error('expected navigationId');
+      }
+      const insightSetKey = firstNav.args.data.navigationId;
       const insightSet = insights.get(insightSetKey);
       if (!insightSet) {
         throw new Error('missing insight set');

front_end/models/trace/types/TraceEvents.ts

@@ -52,8 +52,6 @@ export const enum Phase {
   CLOCK_SYNC = 'c',
 }
 
-export type NonEmptyString = string&{_tag: 'NonEmptyString'};
-
 export function isNestableAsyncPhase(phase: Phase): boolean {
   return phase === Phase.ASYNC_NESTABLE_START || phase === Phase.ASYNC_NESTABLE_END ||
       phase === Phase.ASYNC_NESTABLE_INSTANT;
@@ -596,43 +594,29 @@ export interface Mark extends Event {
   ph: Phase.MARK;
 }
 
-// An unreliable and non-legit navigationStart. See NavigationStartWithUrl
-export interface NavigationStartUnreliable extends Mark {
+export interface NavigationStart extends Mark {
   name: 'navigationStart';
   args: Args&{
-    data?: ArgsData & {
-      /** An empty documentLoaderURL means this navigationStart is unreliable noise and can be ignored. */
-      documentLoaderURL: never,
+    frame: string,
+    data?: ArgsData&{
+      /** Must be non-empty to be valid. An empty documentLoaderURL means the event can be ignored. */
+      documentLoaderURL: string,
       isLoadingMainFrame: boolean,
-      // isOutermostMainFrame was introduced in crrev.com/c/3625434 and exists
-      // because of Fenced Frames
-      // [github.com/WICG/fenced-frame/tree/master/explainer].
-      // Fenced frames introduce a situation where isLoadingMainFrame could be
-      // true for a navigation, but that navigation be within an embedded "main
-      // frame", and therefore it wouldn't be on the top level main frame.
-      // In situations where we need to distinguish that, we can rely on
-      // isOutermostMainFrame, which will only be true for navigations on the
-      // top level main frame.
-
-      // This flag is optional as it was introduced in May 2022; so users
-      // reasonably may import traces from before that date that do not have
-      // this field present.
-      isOutermostMainFrame?: boolean, navigationId: string,
+      navigationId: string,
+      /**
+       * `isOutermostMainFrame` was introduced in crrev.com/c/3625434 and exists because of Fenced Frames
+       * [github.com/WICG/fenced-frame/tree/master/explainer]. Fenced frames introduce a situation where
+       * `isLoadingMainFrame` could be true for a navigation, but that navigation be within an embedded "main frame", and
+       * therefore it wouldn't be on the top level main frame. In situations where we need to distinguish that, we can
+       * rely on `isOutermostMainFrame`, which will only be true for navigations on the top level main frame.
+       * This flag is optional as it was introduced in May 2022; so users reasonably may import traces from before that
+       * date that do not have this field present.
+       */
+      isOutermostMainFrame?: boolean,
       /**
        * @deprecated use documentLoaderURL for navigation events URLs
        */
       url?: string,
-    },
-        frame: string,
-  };
-}
-
-// NavigationStart but definitely has a populated documentLoaderURL
-export interface NavigationStart extends NavigationStartUnreliable {
-  args: NavigationStartUnreliable['args']&{
-    data: NavigationStartUnreliable['args']['data'] & {
-      /** This navigationStart is valid, as the documentLoaderURL isn't empty. */
-      documentLoaderURL: NonEmptyString,
     },
   };
 }
@@ -2050,13 +2034,6 @@ export function isCommitLoad(
   return event.name === 'CommitLoad';
 }
 
-/** @deprecated You probably want `isNavigationStart` instead. */
-export function isNavigationStartUnreliable(
-    event: Event,
-    ): event is NavigationStartUnreliable {
-  return event.name === 'navigationStart';
-}
-
 export function isAnimation(
     event: Event,
     ): event is Animation {
@@ -2210,7 +2187,7 @@ export function isPrePaint(
 
 /** A VALID navigation start (as it has a populated documentLoaderURL) */
 export function isNavigationStart(event: Event): event is NavigationStart {
-  return Boolean(isNavigationStartUnreliable(event) && event.args.data && event.args.data.documentLoaderURL !== '');
+  return event.name === 'navigationStart' && (event as NavigationStart).args?.data?.documentLoaderURL !== '';
 }
 
 export function isMainFrameViewport(