line chart: recreate charts upon fatal renderer errors

tensorboard/webapp/widgets/line_chart_v2/line_chart_component.ng.html

@@ -32,8 +32,13 @@
       [yGridCount]="Y_GRID_COUNT"
       [domDim]="domDimensions.main"
     ></line-chart-grid-view>
-    <svg #chartEl *ngIf="getRendererType() === RendererType.SVG"></svg>
-    <canvas #chartEl *ngIf="getRendererType() === RendererType.WEBGL"></canvas>
+    <ng-container *ngIf="showChartRendererElement">
+      <svg #chartEl *ngIf="getRendererType() === RendererType.SVG"></svg>
+      <canvas
+        #chartEl
+        *ngIf="getRendererType() === RendererType.WEBGL"
+      ></canvas>
+    </ng-container>
     <line-chart-interactive-view
       [seriesData]="seriesData"
       [seriesMetadataMap]="seriesMetadataMap"

tensorboard/webapp/widgets/line_chart_v2/line_chart_component.ts

@@ -18,10 +18,12 @@ import {
   ChangeDetectorRef,
   Component,
   ElementRef,
+  EventEmitter,
   Input,
   OnChanges,
   OnDestroy,
   OnInit,
+  Output,
   SimpleChanges,
   TemplateRef,
   ViewChild,
@@ -162,8 +164,9 @@ export class LineChartComponent
     xAxis: {width: 0, height: 0},
     yAxis: {width: 0, height: 0},
   };
+  showChartRendererElement: boolean = true;
 
-  private lineChart?: Chart;
+  private lineChart: Chart | null = null;
   private isDataUpdated = false;
   private isMetadataUpdated = false;
   private isFixedViewBoxUpdated = false;
@@ -173,6 +176,7 @@ export class LineChartComponent
   // onChanges.
   private isViewBoxChanged = true;
   private scaleUpdated = true;
+  private isRenderingContextLost = false;
 
   constructor(private readonly changeDetector: ChangeDetectorRef) {}
 
@@ -232,6 +236,35 @@ export class LineChartComponent
     this.changeDetector.detectChanges();
   }
 
+  /**
+   * Ensures the renderer is ready, or recovers it if it encountered a loss of
+   * context. This relies on `onContextLost` to set the appropriate flags for
+   * requesting updates.
+   */
+  private recoverRendererIfNeeded() {
+    if (!this.isRenderingContextLost || this.disableUpdate) {
+      return;
+    }
+    // The component's template has an 'ngIf="showChartRendererElement"' we use
+    // to fully replace the DOM element.
+    this.showChartRendererElement = false;
+    this.changeDetector.detectChanges();
+    this.showChartRendererElement = true;
+    this.changeDetector.detectChanges();
+    this.initializeChart();
+
+    // After recreating the renderer element, the next update should re-apply
+    // any existing changes. Keep this in sync with `updateLineChart`.
+    this.scaleUpdated = true;
+    this.isMetadataUpdated = true;
+    this.isDataUpdated = true;
+    this.useDarkModeUpdated = true;
+    this.isFixedViewBoxUpdated = true;
+    this.isViewBoxChanged = true;
+
+    this.isRenderingContextLost = false;
+  }
+
   onViewResize() {
     if (!this.lineChart) return;
 
@@ -289,16 +322,35 @@ export class LineChartComponent
     return false;
   }
 
+  private onContextLost() {
+    // Since context may be lost when the component is hidden or does not need
+    // updates, the re-creation of a new chart renderer happens lazily.
+    this.isRenderingContextLost = true;
+
+    if (this.lineChart) {
+      this.lineChart.dispose();
+      this.lineChart = null;
+    }
+  }
+
+  triggerContextLostForTest() {
+    this.onContextLost();
+  }
+
+  getLineChartForTest(): Chart | null {
+    return this.lineChart;
+  }
+
   private initializeChart() {
     if (this.lineChart) {
-      throw new Error('LineChart should not be initialized multiple times.');
+      this.lineChart.dispose();
     }
 
     const rendererType = this.getRendererType();
-    // Do not yet need to subscribe to the `onDrawEnd`.
     const callbacks: ChartCallbacks = {
+      // Do not yet need to subscribe to the `onDrawEnd`.
       onDrawEnd: () => {},
-      onContextLost: () => {},
+      onContextLost: this.onContextLost.bind(this),
     };
     let params: ChartOptions | null = null;
 
@@ -364,9 +416,12 @@ export class LineChartComponent
   }
 
   /**
-   * Minimally and imperatively updates the chart library depending on prop changed.
+   * Minimally and imperatively updates the chart library depending on prop
+   * changed. When adding new `this.lineChart.set*()` calls, keep this in sync
+   * with `recoverRendererIfNeeded`.
    */
   private updateLineChart() {
+    this.recoverRendererIfNeeded();
     if (!this.lineChart || this.disableUpdate) return;
 
     if (this.scaleUpdated) {

tensorboard/webapp/widgets/line_chart_v2/line_chart_component_test.ts

@@ -21,6 +21,7 @@ import {By} from '@angular/platform-browser';
 
 import {ChartImpl} from './lib/chart';
 import {
+  Chart,
   DataSeries,
   DataSeriesMetadataMap,
   Extent,
@@ -648,4 +649,157 @@ describe('line_chart_v2/line_chart test', () => {
       expect(fixture.debugElement.query(By.css('.dark-mode'))).toBeTruthy();
     });
   });
+
+  describe('onContextLost renderer callback', () => {
+    function expectChartUpdateSpiesToHaveBeenCalledTimes(times: number) {
+      expect(setXScaleTypeSpy).toHaveBeenCalledTimes(times);
+      expect(setYScaleTypeSpy).toHaveBeenCalledTimes(times);
+      expect(updateMetadataSpy).toHaveBeenCalledTimes(times);
+      expect(updateDataSpy).toHaveBeenCalledTimes(times);
+      expect(updateViewBoxSpy).toHaveBeenCalledTimes(times);
+    }
+
+    function didChartRendererRecover(
+      fixture: ComponentFixture<TestableComponent>,
+      chartBefore: Chart | null
+    ): boolean {
+      const chartAfter = fixture.componentInstance.chart.getLineChartForTest();
+      return chartBefore !== chartAfter && !!chartAfter;
+    }
+
+    it('does not recover the renderer by default', () => {
+      const fixture = createComponent({
+        seriesData: [buildSeries({id: 'foo'})],
+        seriesMetadataMap: {foo: buildMetadata({id: 'foo', visible: true})},
+        yScaleType: ScaleType.LINEAR,
+      });
+      fixture.detectChanges();
+      const chartBefore = fixture.componentInstance.chart.getLineChartForTest();
+
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1); // Initial render.
+
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(false);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1);
+    });
+
+    it('does not recover the renderer when disabling updates', () => {
+      const fixture = createComponent({
+        seriesData: [buildSeries({id: 'foo'})],
+        seriesMetadataMap: {foo: buildMetadata({id: 'foo', visible: true})},
+        yScaleType: ScaleType.LINEAR,
+      });
+      fixture.componentInstance.disableUpdate = false;
+      fixture.detectChanges();
+      const chartBefore = fixture.componentInstance.chart.getLineChartForTest();
+
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1); // Initial render.
+
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.detectChanges();
+
+      fixture.componentInstance.disableUpdate = true;
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(false);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1);
+    });
+
+    it('recovers the renderer when enabling updates', () => {
+      const fixture = createComponent({
+        seriesData: [buildSeries({id: 'foo'})],
+        seriesMetadataMap: {foo: buildMetadata({id: 'foo', visible: true})},
+        yScaleType: ScaleType.LINEAR,
+      });
+      fixture.componentInstance.disableUpdate = true;
+      fixture.detectChanges();
+      const chartBefore = fixture.componentInstance.chart.getLineChartForTest();
+
+      expectChartUpdateSpiesToHaveBeenCalledTimes(0); // No initial render.
+
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.detectChanges();
+
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(0);
+
+      // Updates now enabled.
+      fixture.componentInstance.disableUpdate = false;
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(true);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1);
+    });
+
+    it('recovers the renderer when updating the chart', () => {
+      const fixture = createComponent({
+        seriesData: [buildSeries({id: 'foo'})],
+        seriesMetadataMap: {foo: buildMetadata({id: 'foo', visible: true})},
+        yScaleType: ScaleType.LINEAR,
+      });
+      fixture.detectChanges();
+      const chartBefore = fixture.componentInstance.chart.getLineChartForTest();
+
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1); // Initial render.
+
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(false);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1);
+
+      fixture.componentInstance.yScaleType = ScaleType.LOG10;
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(true);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(2);
+    });
+
+    it('does not recover more than once if not necessary', () => {
+      const fixture = createComponent({
+        seriesData: [buildSeries({id: 'foo'})],
+        seriesMetadataMap: {foo: buildMetadata({id: 'foo', visible: true})},
+        yScaleType: ScaleType.LINEAR,
+      });
+      fixture.componentInstance.disableUpdate = true;
+      fixture.detectChanges();
+      const chartBefore = fixture.componentInstance.chart.getLineChartForTest();
+
+      expectChartUpdateSpiesToHaveBeenCalledTimes(0); // No initial render.
+
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.componentInstance.chart.triggerContextLostForTest();
+      fixture.detectChanges();
+
+      // Updates enabled the first time.
+      fixture.componentInstance.disableUpdate = false;
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(true);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1);
+
+      // Updates re-enabled more times.
+      fixture.componentInstance.disableUpdate = true;
+      fixture.detectChanges();
+      fixture.componentInstance.disableUpdate = false;
+      fixture.detectChanges();
+      fixture.componentInstance.disableUpdate = true;
+      fixture.detectChanges();
+      fixture.componentInstance.disableUpdate = false;
+      fixture.detectChanges();
+
+      expect(didChartRendererRecover(fixture, chartBefore)).toBe(true);
+      expect(disposeSpy).toHaveBeenCalledTimes(1);
+      expectChartUpdateSpiesToHaveBeenCalledTimes(1);
+    });
+  });
 });