Only send "analysis.analyzedFiles" notification when the set of files…

… changes.

R=jwren@google.com

Review URL: https://codereview.chromium.org//1233003002 .

pkg/analysis_server/lib/src/analysis_server.dart

@@ -256,6 +256,15 @@ class AnalysisServer {
    */
   List<Plugin> userDefinedPlugins;
 
+  /**
+   * If the "analysis.analyzedFiles" notification is currently being subscribed
+   * to (see [generalAnalysisServices]), and at least one such notification has
+   * been sent since the subscription was enabled, the set of analyzed files
+   * that was delivered in the most recently sent notification.  Otherwise
+   * `null`.
+   */
+  Set<String> prevAnalyzedFiles;
+
   /**
    * Initialize a newly created server to receive requests from and send
    * responses to the given [channel].
@@ -970,6 +979,10 @@ class AnalysisServer {
             .contains(GeneralAnalysisService.ANALYZED_FILES) &&
         isAnalysisComplete()) {
       sendAnalysisNotificationAnalyzedFiles(this);
+    } else if (!newServices.contains(GeneralAnalysisService.ANALYZED_FILES) &&
+        generalAnalysisServices
+            .contains(GeneralAnalysisService.ANALYZED_FILES)) {
+      prevAnalyzedFiles = null;
     }
     generalAnalysisServices = newServices;
   }

pkg/analysis_server/lib/src/operation/operation_analysis.dart

@@ -101,11 +101,24 @@ void scheduleNotificationOperations(AnalysisServer server, String file,
 
 void sendAnalysisNotificationAnalyzedFiles(AnalysisServer server) {
   _sendNotification(server, () {
+    // TODO(paulberry): if it proves to be too inefficient to recompute the set
+    // of analyzed files each time analysis is complete, consider modifying the
+    // analysis engine to update this set incrementally as analysis is
+    // performed.
     LibraryDependencyCollector collector =
         new LibraryDependencyCollector(server.getAnalysisContexts().toList());
-    Set<String> directories = collector.collectLibraryDependencies();
+    Set<String> analyzedFiles = collector.collectLibraryDependencies();
+    Set<String> prevAnalyzedFiles = server.prevAnalyzedFiles;
+    if (prevAnalyzedFiles != null &&
+        prevAnalyzedFiles.length == analyzedFiles.length &&
+        prevAnalyzedFiles.difference(analyzedFiles).isEmpty) {
+      // No change to the set of analyzed files.  No need to send another
+      // notification.
+      return;
+    }
+    server.prevAnalyzedFiles = analyzedFiles;
     protocol.AnalysisAnalyzedFilesParams params =
-        new protocol.AnalysisAnalyzedFilesParams(directories.toList());
+        new protocol.AnalysisAnalyzedFilesParams(analyzedFiles.toList());
     server.sendNotification(params.toNotification());
   });
 }

pkg/analysis_server/test/analysis/notification_analyzedFiles_test.dart

@@ -12,6 +12,7 @@ import 'package:test_reflective_loader/test_reflective_loader.dart';
 import 'package:unittest/unittest.dart';
 
 import '../analysis_abstract.dart';
+import '../mocks.dart';
 
 main() {
   groupSep = ' | ';
@@ -21,8 +22,10 @@ main() {
 @reflectiveTest
 class AnalysisNotificationAnalyzedFilesTest extends AbstractAnalysisTest {
   List<String> analyzedFiles;
+  bool analyzedFilesReceived = false;
 
   void assertHasFile(String filePath) {
+    expect(analyzedFilesReceived, isTrue);
     expect(analyzedFiles, contains(filePath));
   }
 
@@ -35,6 +38,7 @@ class AnalysisNotificationAnalyzedFilesTest extends AbstractAnalysisTest {
     if (notification.event == ANALYSIS_ANALYZED_FILES) {
       AnalysisAnalyzedFilesParams params =
           new AnalysisAnalyzedFilesParams.fromNotification(notification);
+      analyzedFilesReceived = true;
       analyzedFiles = params.directories;
     }
   }
@@ -55,12 +59,60 @@ class A {}
     });
   }
 
-  test_definedInInterface_ofInterface() {
+  test_beforeAnalysis() {
     addTestFile('''
 class A {}
 ''');
     return prepareAnalyzedFiles().then((_) {
       assertHasFile(testFile);
     });
   }
+
+  test_insignificant_change() async {
+    // Making a change that doesn't affect the set of reachable files should
+    // not trigger the notification to be re-sent.
+    addTestFile('class A {}');
+    await prepareAnalyzedFiles();
+    await waitForTasksFinished();
+    expect(analyzedFilesReceived, isTrue);
+    analyzedFilesReceived = false;
+    modifyTestFile('class B {}');
+    await pumpEventQueue();
+    await waitForTasksFinished();
+    expect(analyzedFilesReceived, isFalse);
+  }
+
+  test_resubscribe_no_changes() async {
+    // Unsubscribing and resubscribing should cause the notification to be
+    // re-sent, even if nothing has changed.
+    addTestFile('class A {}');
+    await prepareAnalyzedFiles();
+    await waitForTasksFinished();
+    expect(analyzedFilesReceived, isTrue);
+    unsubscribeAnalyzedFiles();
+    analyzedFilesReceived = false;
+    await prepareAnalyzedFiles();
+    expect(analyzedFilesReceived, isTrue);
+    assertHasFile(testFile);
+  }
+
+  test_significant_change() async {
+    // Making a change that *does* affect the set of reachable files should
+    // trigger the notification to be re-sent.
+    addTestFile('class A {}');
+    addFile('/foo.dart', 'library foo');
+    await prepareAnalyzedFiles();
+    await waitForTasksFinished();
+    expect(analyzedFilesReceived, isTrue);
+    analyzedFilesReceived = false;
+    modifyTestFile('import "/foo.dart";');
+    await pumpEventQueue();
+    await waitForTasksFinished();
+    expect(analyzedFilesReceived, isTrue);
+    assertHasFile('/foo.dart');
+  }
+
+  void unsubscribeAnalyzedFiles() {
+    removeGeneralAnalysisSubscription(GeneralAnalysisService.ANALYZED_FILES);
+  }
 }

pkg/analysis_server/test/analysis_abstract.dart

@@ -156,6 +156,13 @@ class AbstractAnalysisTest {
     }
   }
 
+  void removeGeneralAnalysisSubscription(GeneralAnalysisService service) {
+    generalServices.remove(service);
+    Request request = new AnalysisSetGeneralSubscriptionsParams(generalServices)
+        .toRequest('0');
+    handleSuccessfulRequest(request);
+  }
+
   void setUp() {
     serverChannel = new MockServerChannel();
     resourceProvider = new MemoryResourceProvider();