Port final trust fixes for release (#12965)

* Only allow Enter / NumpadEnter w/o ctrl/shift/alt (#12939)

* Send telemetry for notebook trust prompt selections (#12964)

* Fixes for persisting trust (#12950)

Author: joyceerhl

src/client/datascience/constants.ts

@@ -368,7 +368,11 @@ export enum Telemetry {
     RunByLineStart = 'DATASCIENCE.RUN_BY_LINE',
     RunByLineStep = 'DATASCIENCE.RUN_BY_LINE_STEP',
     RunByLineStop = 'DATASCIENCE.RUN_BY_LINE_STOP',
-    RunByLineVariableHover = 'DATASCIENCE.RUN_BY_LINE_VARIABLE_HOVER'
+    RunByLineVariableHover = 'DATASCIENCE.RUN_BY_LINE_VARIABLE_HOVER',
+    TrustAllNotebooks = 'DATASCIENCE.TRUST_ALL_NOTEBOOKS',
+    TrustNotebook = 'DATASCIENCE.TRUST_NOTEBOOK',
+    DoNotTrustNotebook = 'DATASCIENCE.DO_NOT_TRUST_NOTEBOOK',
+    NotebookTrustPromptShown = 'DATASCIENCE.NOTEBOOK_TRUST_PROMPT_SHOWN'
 }
 
 export enum NativeKeyboardCommandTelemetry {

src/client/datascience/interactive-ipynb/nativeEditorStorage.ts

@@ -282,10 +282,10 @@ export class NativeEditorStorage implements INotebookStorage {
             const dirtyContents = skipDirtyContents ? undefined : await this.getStoredContents(file, backupId);
             if (dirtyContents) {
                 // This means we're dirty. Indicate dirty and load from this content
-                return this.loadContents(file, dirtyContents, true, contents, forVSCodeNotebook);
+                return this.loadContents(file, dirtyContents, true, forVSCodeNotebook);
             } else {
                 // Load without setting dirty
-                return this.loadContents(file, contents, undefined, undefined, forVSCodeNotebook);
+                return this.loadContents(file, contents, undefined, forVSCodeNotebook);
             }
         } catch (ex) {
             // May not exist at this time. Should always have a single cell though
@@ -308,7 +308,6 @@ export class NativeEditorStorage implements INotebookStorage {
         file: Uri,
         contents: string | undefined,
         isInitiallyDirty = false,
-        trueContents?: string,
         forVSCodeNotebook?: boolean
     ) {
         // tslint:disable-next-line: no-any
@@ -348,18 +347,9 @@ export class NativeEditorStorage implements INotebookStorage {
         }
         const pythonNumber = json ? await this.extractPythonMainVersion(json) : 3;
 
-        /* As an optimization, we don't call trustNotebook for hot exit, since our hot exit backup code gets called by VS
-        Code whenever the notebook model changes. This means it's called very often, perhaps even as often as autosave.
-        Instead, when loading a file that is dirty, we check if the actual file contents on disk are trusted. If so, we treat
-        the dirty contents as trusted as well. */
-        const contentsToCheck = isInitiallyDirty && trueContents !== undefined ? trueContents : contents;
-        const isTrusted =
-            contents === undefined || isUntitledFile(file)
-                ? true // If no contents or untitled, this is a newly created file, so it should be trusted
-                : await this.trustService.isNotebookTrusted(file, contentsToCheck!);
-        return this.factory.createModel(
+        const model = this.factory.createModel(
             {
-                trusted: isTrusted,
+                trusted: true,
                 file,
                 cells: remapped,
                 notebookJson: json,
@@ -369,6 +359,23 @@ export class NativeEditorStorage implements INotebookStorage {
             },
             forVSCodeNotebook
         );
+
+        // If no contents or untitled, this is a newly created file
+        // If dirty, that means it's been edited before in our extension
+        if (contents !== undefined && !isUntitledFile(file) && !isInitiallyDirty) {
+            const isNotebookTrusted = await this.trustService.isNotebookTrusted(file, model.getContent());
+            if (isNotebookTrusted !== model.isTrusted) {
+                model.update({
+                    source: 'user',
+                    kind: 'updateTrust',
+                    oldDirty: model.isDirty,
+                    newDirty: model.isDirty,
+                    isNotebookTrusted
+                });
+            }
+        }
+
+        return model;
     }
 
     private getStaticStorageKey(file: Uri): string {

src/client/datascience/interactive-ipynb/trustCommandHandler.ts

@@ -13,7 +13,8 @@ import '../../common/extensions';
 import { IDisposableRegistry, IExperimentService } from '../../common/types';
 import { swallowExceptions } from '../../common/utils/decorators';
 import { DataScience } from '../../common/utils/localize';
-import { Commands } from '../constants';
+import { sendTelemetryEvent } from '../../telemetry';
+import { Commands, Telemetry } from '../constants';
 import { INotebookStorageProvider } from '../interactive-ipynb/notebookStorageProvider';
 import { INotebookEditorProvider, ITrustService } from '../types';
 
@@ -57,10 +58,12 @@ export class TrustCommandHandler implements IExtensionSingleActivationService {
             DataScience.doNotTrustNotebook(),
             DataScience.trustAllNotebooks()
         );
+        sendTelemetryEvent(Telemetry.NotebookTrustPromptShown);
 
         switch (selection) {
             case DataScience.trustAllNotebooks():
                 commands.executeCommand('workbench.action.openSettings', 'python.dataScience.alwaysTrustNotebooks');
+                sendTelemetryEvent(Telemetry.TrustAllNotebooks);
                 break;
             case DataScience.trustNotebook():
                 // Update model trust
@@ -73,6 +76,10 @@ export class TrustCommandHandler implements IExtensionSingleActivationService {
                 });
                 const contents = model.getContent();
                 await this.trustService.trustNotebook(model.file, contents);
+                sendTelemetryEvent(Telemetry.TrustNotebook);
+                break;
+            case DataScience.doNotTrustNotebook():
+                sendTelemetryEvent(Telemetry.DoNotTrustNotebook);
                 break;
             default:
                 break;

src/client/datascience/notebookStorage/baseModel.ts

@@ -196,12 +196,8 @@ export abstract class BaseNotebookModel implements INotebookModel {
         this.ensureNotebookJson();
 
         // Reuse our original json except for the cells.
-        const json = {
-            cells: this.cells.map((c) => pruneCell(c.data)),
-            metadata: this.notebookJson.metadata,
-            nbformat: this.notebookJson.nbformat,
-            nbformat_minor: this.notebookJson.nbformat_minor
-        };
+        const json = { ...this.notebookJson };
+        json.cells = this.cells.map((c) => pruneCell(c.data));
         return JSON.stringify(json, null, this.indentAmount);
     }
 }

src/client/telemetry/index.ts

@@ -2192,10 +2192,20 @@ export interface IEventNamePropertyMapping {
     [Telemetry.StartPageOpenFileBrowser]: never | undefined;
     [Telemetry.StartPageOpenFolder]: never | undefined;
     [Telemetry.StartPageOpenWorkspace]: never | undefined;
+
+    // Run by line events
     [Telemetry.RunByLineStart]: never | undefined;
     [Telemetry.RunByLineStep]: never | undefined;
     [Telemetry.RunByLineStop]: never | undefined;
     [Telemetry.RunByLineVariableHover]: never | undefined;
+
+    // Trusted notebooks events
+    [Telemetry.NotebookTrustPromptShown]: never | undefined;
+    [Telemetry.TrustNotebook]: never | undefined;
+    [Telemetry.TrustAllNotebooks]: never | undefined;
+    [Telemetry.DoNotTrustNotebook]: never | undefined;
+
+    // Native notebooks events
     [VSCodeNativeTelemetry.AddCell]: never | undefined;
     [VSCodeNativeTelemetry.DeleteCell]: never | undefined;
     [VSCodeNativeTelemetry.MoveCell]: never | undefined;

src/datascience-ui/native-editor/nativeCell.tsx

@@ -892,8 +892,7 @@ export function getConnectedNativeCell() {
 
 function isCellNavigationKeyboardEvent(e: IKeyboardEvent) {
     return (
-        e.code === 'Enter' ||
-        e.code === 'NumpadEnter' ||
+        ((e.code === 'Enter' || e.code === 'NumpadEnter') && !e.shiftKey && !e.ctrlKey && !e.altKey) ||
         e.code === 'ArrowUp' ||
         e.code === 'k' ||
         e.code === 'ArrowDown' ||