Don't report removed sources as changed in CacheConsistencyValidatorImpl.

Also I believe we don't need this check anymore.
context.getLibrariesContaining(source).isEmpty &&
context.getLibrariesDependingOn(source).isEmpty)

1. It was not used in applyChanges(), which could probably cause losing
some dependencies.

2. Since https://codereview.chromium.org/2172143003 we record that
there are dependencies on the removed, so not existing, source. So,
when the source is added again, _sourceAvailable() will invalidate
its CONTENT and the whole transitive closure of its users.

R=brianwilkerson@google.com
BUG=

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

pkg/analysis_server/lib/src/status/get_handler.dart

@@ -827,17 +827,17 @@ class GetHandler {
                 '<table style="border-collapse: separate; border-spacing: 10px 5px;">');
             _writeRow(buffer, ['Name', 'Count'], header: true);
             _writeRow(buffer, [
-              'Modified',
+              'Changed',
               PerformanceStatistics
-                  .cacheConsistencyValidationStatistics.numOfModified
+                  .cacheConsistencyValidationStatistics.numOfChanged
             ], classes: [
               null,
               "right"
             ]);
             _writeRow(buffer, [
-              'Deleted',
+              'Removed',
               PerformanceStatistics
-                  .cacheConsistencyValidationStatistics.numOfDeleted
+                  .cacheConsistencyValidationStatistics.numOfRemoved
             ], classes: [
               null,
               "right"

pkg/analyzer/lib/src/context/cache.dart

@@ -697,10 +697,6 @@ class CacheEntry {
 //      if (deltaResult == null) {
 //        String indent = '  ' * level;
 //        print('[$id]$indent invalidate $descriptor for $target');
-//        if ('$descriptor for $target' ==
-//            'READY_LIBRARY_ELEMENT2 for /Users/scheglov/tmp/limited-invalidation/async/lib/async.dart') {
-//          print('interesting');
-//        }
 //      }
     }
     // Invalidate results that depend on this result.

pkg/analyzer/lib/src/context/context.dart

@@ -2072,9 +2072,9 @@ class CacheConsistencyValidatorImpl implements CacheConsistencyValidator {
 
   @override
   bool sourceModificationTimesComputed(List<Source> sources, List<int> times) {
-    int consistencyCheckStart = JavaSystem.nanoTime();
+    Stopwatch timer = new Stopwatch()..start();
     HashSet<Source> changedSources = new HashSet<Source>();
-    HashSet<Source> missingSources = new HashSet<Source>();
+    HashSet<Source> removedSources = new HashSet<Source>();
     for (int i = 0; i < sources.length; i++) {
       Source source = sources[i];
       // When a source is in the content cache,
@@ -2091,16 +2091,15 @@ class CacheConsistencyValidatorImpl implements CacheConsistencyValidator {
       // Compare with the modification time in the cache entry.
       CacheEntry entry = context._privatePartition.get(source);
       if (entry != null) {
-        if (sourceTime != entry.modificationTime) {
-          changedSources.add(source);
-          PerformanceStatistics
-              .cacheConsistencyValidationStatistics.numOfModified++;
-        }
-        if (entry.exception != null) {
+        if (entry.modificationTime != sourceTime) {
           if (sourceTime == -1) {
-            missingSources.add(source);
+            removedSources.add(source);
             PerformanceStatistics
-                .cacheConsistencyValidationStatistics.numOfModified++;
+                .cacheConsistencyValidationStatistics.numOfRemoved++;
+          } else {
+            changedSources.add(source);
+            PerformanceStatistics
+                .cacheConsistencyValidationStatistics.numOfChanged++;
           }
         }
       }
@@ -2108,35 +2107,23 @@ class CacheConsistencyValidatorImpl implements CacheConsistencyValidator {
     for (Source source in changedSources) {
       context._sourceChanged(source);
     }
-    int removalCount = 0;
-    for (Source source in missingSources) {
-      if (context.getLibrariesContaining(source).isEmpty &&
-          context.getLibrariesDependingOn(source).isEmpty) {
-        context._removeFromCache(source);
-        removalCount++;
-      }
+    for (Source source in removedSources) {
+      context._sourceRemoved(source);
     }
-    int consistencyCheckEnd = JavaSystem.nanoTime();
-    if (changedSources.length > 0 || missingSources.length > 0) {
+    if (changedSources.length > 0 || removedSources.length > 0) {
       StringBuffer buffer = new StringBuffer();
       buffer.write("Consistency check took ");
-      buffer.write((consistencyCheckEnd - consistencyCheckStart) / 1000000.0);
+      buffer.write(timer.elapsedMilliseconds);
       buffer.writeln(" ms and found");
       buffer.write("  ");
       buffer.write(changedSources.length);
-      buffer.writeln(" inconsistent entries");
+      buffer.writeln(" changed sources");
       buffer.write("  ");
-      buffer.write(missingSources.length);
-      buffer.write(" missing sources (");
-      buffer.write(removalCount);
-      buffer.writeln(" removed");
-      for (Source source in missingSources) {
-        buffer.write("    ");
-        buffer.writeln(source.fullName);
-      }
+      buffer.write(removedSources.length);
+      buffer.write(" removed sources.");
       context._logInformation(buffer.toString());
     }
-    return changedSources.length > 0;
+    return changedSources.isNotEmpty || removedSources.isNotEmpty;
   }
 }
 

pkg/analyzer/lib/src/generated/engine.dart

@@ -1190,6 +1190,11 @@ class AnalysisOptionsImpl implements AnalysisOptions {
   static const int ENABLE_STRONG_MODE_HINTS_FLAG = 0x20;
   static const int ENABLE_SUPER_MIXINS_FLAG = 0x40;
 
+  /**
+   * The default list of non-nullable type names.
+   */
+  static const List<String> NONNULLABLE_TYPES = const <String>[];
+
   /**
    * A predicate indicating whether analysis is to parse and analyze function
    * bodies.
@@ -1333,11 +1338,6 @@ class AnalysisOptionsImpl implements AnalysisOptions {
    */
   bool implicitDynamic = true;
 
-  /**
-   * The default list of non-nullable type names.
-   */
-  static const List<String> NONNULLABLE_TYPES = const <String>[];
-
   /**
    * Initialize a newly created set of analysis options to have their default
    * values.
@@ -1560,18 +1560,26 @@ class ApplyChangesStatus {
  */
 class CacheConsistencyValidationStatistics {
   /**
-   * Number of sources which were modified, but the context was not notified
+   * Number of sources which were changed, but the context was not notified
    * about it, so this fact was detected only during cache consistency
    * validation.
    */
-  int numOfModified = 0;
+  int numOfChanged = 0;
 
   /**
-   * Number of sources which were deleted, but the context was not notified
+   * Number of sources which stopped existing, but the context was not notified
    * about it, so this fact was detected only during cache consistency
    * validation.
    */
-  int numOfDeleted = 0;
+  int numOfRemoved = 0;
+
+  /**
+   * Reset all counters.
+   */
+  void reset() {
+    numOfChanged = 0;
+    numOfRemoved = 0;
+  }
 }
 
 /**

pkg/analyzer/test/src/context/context_test.dart

@@ -455,8 +455,42 @@ import 'libB.dart';''';
     });
   }
 
-  void test_cacheConsistencyValidator_computed() {
+  void test_applyChanges_removeUsedLibrary_addAgain() {
+    String codeA = r'''
+import 'b.dart';
+B b = null;
+''';
+    String codeB = r'''
+class B {}
+''';
+    Source a = addSource('/a.dart', codeA);
+    Source b = addSource('/b.dart', codeB);
+    CacheState getErrorsState(Source source) =>
+        context.analysisCache.getState(source, LIBRARY_ERRORS_READY);
+    _performPendingAnalysisTasks();
+    expect(context.getErrors(a).errors, hasLength(0));
+    // Remove b.dart - errors in a.dart are invalidated and recomputed.
+    // Now a.dart has an error.
+    _removeSource(b);
+    expect(getErrorsState(a), CacheState.INVALID);
+    _performPendingAnalysisTasks();
+    expect(getErrorsState(a), CacheState.VALID);
+    expect(context.getErrors(a).errors, hasLength(isPositive));
+    // Add b.dart - errors in a.dart are invalidated and recomputed.
+    // The reason is that a.dart adds dependencies on (not existing) b.dart
+    // results in cache.
+    // Now a.dart does not have errors.
+    addSource('/b.dart', codeB);
+    expect(getErrorsState(a), CacheState.INVALID);
+    _performPendingAnalysisTasks();
+    expect(getErrorsState(a), CacheState.VALID);
+    expect(context.getErrors(a).errors, hasLength(0));
+  }
+
+  void test_cacheConsistencyValidator_computed_deleted() {
     CacheConsistencyValidator validator = context.cacheConsistencyValidator;
+    var stat = PerformanceStatistics.cacheConsistencyValidationStatistics;
+    stat.reset();
     // Add sources.
     MemoryResourceProvider resourceProvider = new MemoryResourceProvider();
     String path1 = '/test1.dart';
@@ -470,6 +504,45 @@ import 'libB.dart';''';
         validator.sourceModificationTimesComputed([source1, source2],
             [source1.modificationStamp, source2.modificationStamp]),
         isFalse);
+    expect(stat.numOfChanged, 0);
+    expect(stat.numOfRemoved, 0);
+    // Add overlay
+    context.setContents(source1, '// 1-2');
+    expect(
+        validator.sourceModificationTimesComputed(
+            [source1, source2], [-1, source2.modificationStamp]),
+        isFalse);
+    context.setContents(source1, null);
+    expect(stat.numOfChanged, 0);
+    expect(stat.numOfRemoved, 0);
+    // Different modification times.
+    expect(
+        validator.sourceModificationTimesComputed(
+            [source1, source2], [-1, source2.modificationStamp]),
+        isTrue);
+    expect(stat.numOfChanged, 0);
+    expect(stat.numOfRemoved, 1);
+  }
+
+  void test_cacheConsistencyValidator_computed_modified() {
+    CacheConsistencyValidator validator = context.cacheConsistencyValidator;
+    var stat = PerformanceStatistics.cacheConsistencyValidationStatistics;
+    stat.reset();
+    // Add sources.
+    MemoryResourceProvider resourceProvider = new MemoryResourceProvider();
+    String path1 = '/test1.dart';
+    String path2 = '/test2.dart';
+    Source source1 = resourceProvider.newFile(path1, '// 1-1').createSource();
+    Source source2 = resourceProvider.newFile(path2, '// 2-1').createSource();
+    context.applyChanges(
+        new ChangeSet()..addedSource(source1)..addedSource(source2));
+    // Same modification times.
+    expect(
+        validator.sourceModificationTimesComputed([source1, source2],
+            [source1.modificationStamp, source2.modificationStamp]),
+        isFalse);
+    expect(stat.numOfChanged, 0);
+    expect(stat.numOfRemoved, 0);
     // Add overlay
     context.setContents(source1, '// 1-2');
     expect(
@@ -477,11 +550,15 @@ import 'libB.dart';''';
             [source1.modificationStamp + 1, source2.modificationStamp]),
         isFalse);
     context.setContents(source1, null);
+    expect(stat.numOfChanged, 0);
+    expect(stat.numOfRemoved, 0);
     // Different modification times.
     expect(
         validator.sourceModificationTimesComputed([source1, source2],
             [source1.modificationStamp + 1, source2.modificationStamp]),
         isTrue);
+    expect(stat.numOfChanged, 1);
+    expect(stat.numOfRemoved, 0);
   }
 
   void test_cacheConsistencyValidator_getSources() {
@@ -2821,9 +2898,8 @@ List<A> foo() => [];
     _performPendingAnalysisTasks();
     expect(context.getErrors(b).errors, hasLength(0));
     // Add @deprecated annotation.
-    // b.dart could have valid resolution, because A is still A,
-    // but deprecated hints are reported in resolved. So, everything in b.dart
-    // is invalid.
+    // b.dart has valid resolution, because A is still A, so only errors are
+    // invalidated.
     context.setContents(
         a,
         r'''
@@ -2860,9 +2936,8 @@ List<A> foo() => [];
     _performPendingAnalysisTasks();
     expect(context.getErrors(b).errors, hasLength(1));
     // Add @deprecated annotation.
-    // b.dart could have valid resolution, because A is still A,
-    // but deprecated hints are reported in resolved. So, everything in b.dart
-    // is invalid.
+    // b.dart has valid resolution, because A is still A, so only errors are
+    // invalidated.
     context.setContents(
         a,
         r'''