[web] [fix] Cache resource data only if the fetching succeed (#103816)

dev/bots/service_worker_test.dart

@@ -20,6 +20,7 @@ final String _testAppDirectory = path.join(_flutterRoot, 'dev', 'integration_tes
 final String _testAppWebDirectory = path.join(_testAppDirectory, 'web');
 final String _appBuildDirectory = path.join(_testAppDirectory, 'build', 'web');
 final String _target = path.join('lib', 'service_worker_test.dart');
+final String _targetWithCachedResources = path.join('lib', 'service_worker_test_cached_resources.dart');
 final String _targetPath = path.join(_testAppDirectory, _target);
 
 enum ServiceWorkerTestType {
@@ -30,9 +31,12 @@ enum ServiceWorkerTestType {
 
 // Run a web service worker test as a standalone Dart program.
 Future<void> main() async {
-  await runWebServiceWorkerTest(headless: false, testType: ServiceWorkerTestType.withFlutterJs);
   await runWebServiceWorkerTest(headless: false, testType: ServiceWorkerTestType.withoutFlutterJs);
+  await runWebServiceWorkerTest(headless: false, testType: ServiceWorkerTestType.withFlutterJs);
   await runWebServiceWorkerTest(headless: false, testType: ServiceWorkerTestType.withFlutterJsShort);
+  await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withoutFlutterJs);
+  await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withFlutterJs);
+  await runWebServiceWorkerTestWithCachingResources(headless: false, testType: ServiceWorkerTestType.withFlutterJsShort);
 }
 
 Future<void> _setAppVersion(int version) async {
@@ -61,7 +65,7 @@ String _testTypeToIndexFile(ServiceWorkerTestType type) {
   return indexFile;
 }
 
-Future<void> _rebuildApp({ required int version, required ServiceWorkerTestType testType }) async {
+Future<void> _rebuildApp({ required int version, required ServiceWorkerTestType testType, required String target }) async {
   await _setAppVersion(version);
   await runCommand(
     _flutter,
@@ -78,7 +82,7 @@ Future<void> _rebuildApp({ required int version, required ServiceWorkerTestType
   );
   await runCommand(
     _flutter,
-    <String>['build', 'web', '--profile', '-t', _target],
+    <String>['build', 'web', '--profile', '-t', target],
     workingDirectory: _testAppDirectory,
     environment: <String, String>{
       'FLUTTER_WEB': 'true',
@@ -86,6 +90,32 @@ Future<void> _rebuildApp({ required int version, required ServiceWorkerTestType
   );
 }
 
+void _expectRequestCounts(
+    Map<String, int> expectedCounts,
+    Map<String, int> requestedPathCounts,
+) {
+  expect(requestedPathCounts, expectedCounts);
+  requestedPathCounts.clear();
+}
+
+Future<void> _waitForAppToLoad(
+    Map<String, int> waitForCounts,
+    Map<String, int> requestedPathCounts,
+    AppServer? server
+) async {
+  print('Waiting for app to load $waitForCounts');
+  await Future.any(<Future<Object?>>[
+    () async {
+      while (!waitForCounts.entries.every((MapEntry<String, int> entry) => (requestedPathCounts[entry.key] ?? 0) >= entry.value)) {
+        await Future<void>.delayed(const Duration(milliseconds: 100));
+      }
+    }(),
+    server!.onChromeError.then((String error) {
+      throw Exception('Chrome error: $error');
+    }),
+  ]);
+}
+
 /// A drop-in replacement for `package:test` expect that can run outside the
 /// test zone.
 void expect(Object? actual, Object? expected) {
@@ -105,25 +135,12 @@ Future<void> runWebServiceWorkerTest({
   required ServiceWorkerTestType testType,
 }) async {
   final Map<String, int> requestedPathCounts = <String, int>{};
-  void expectRequestCounts(Map<String, int> expectedCounts) {
+  void expectRequestCounts(Map<String, int> expectedCounts) =>
-    expect(requestedPathCounts, expectedCounts);
+      _expectRequestCounts(expectedCounts, requestedPathCounts);
-    requestedPathCounts.clear();
-  }
 
   AppServer? server;
-  Future<void> waitForAppToLoad(Map<String, int> waitForCounts) async {
+  Future<void> waitForAppToLoad(Map<String, int> waitForCounts) async =>
-    print('Waiting for app to load $waitForCounts');
+      _waitForAppToLoad(waitForCounts, requestedPathCounts, server);
-    await Future.any(<Future<Object?>>[
-      () async {
-        while (!waitForCounts.entries.every((MapEntry<String, int> entry) => (requestedPathCounts[entry.key] ?? 0) >= entry.value)) {
-          await Future<void>.delayed(const Duration(milliseconds: 100));
-        }
-      }(),
-      server!.onChromeError.then((String error) {
-        throw Exception('Chrome error: $error');
-      }),
-    ]);
-  }
 
   String? reportedVersion;
 
@@ -174,7 +191,7 @@ Future<void> runWebServiceWorkerTest({
     /////
     // Attempt to load a different version of the service worker!
     /////
-    await _rebuildApp(version: 1, testType: testType);
+    await _rebuildApp(version: 1, testType: testType, target: _target);
 
     print('Call update() on the current web worker');
     await startAppServer(cacheControl: 'max-age=0');
@@ -195,7 +212,7 @@ Future<void> runWebServiceWorkerTest({
     expect(reportedVersion, '1');
     reportedVersion = null;
 
-    await _rebuildApp(version: 2, testType: testType);
+    await _rebuildApp(version: 2, testType: testType, target: _target);
 
     await server!.chrome.reloadPage(ignoreCache: true);
     await waitForAppToLoad(<String, int>{
@@ -212,7 +229,7 @@ Future<void> runWebServiceWorkerTest({
     //////////////////////////////////////////////////////
     // Caching server
     //////////////////////////////////////////////////////
-    await _rebuildApp(version: 1, testType: testType);
+    await _rebuildApp(version: 1, testType: testType, target: _target);
 
     print('With cache: test first page load');
     await startAppServer(cacheControl: 'max-age=3600');
@@ -232,6 +249,7 @@ Future<void> runWebServiceWorkerTest({
       'flutter_service_worker.js': 1,
       'assets/FontManifest.json': 1,
       'assets/AssetManifest.json': 1,
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
       'CLOSE': 1,
       // In headless mode Chrome does not load 'manifest.json' and 'favicon.ico'.
       if (!headless)
@@ -258,7 +276,7 @@ Future<void> runWebServiceWorkerTest({
     reportedVersion = null;
 
     print('With cache: test page reload after rebuild');
-    await _rebuildApp(version: 2, testType: testType);
+    await _rebuildApp(version: 2, testType: testType, target: _target);
 
     // Since we're caching, we need to ignore cache when reloading the page.
     await server!.chrome.reloadPage(ignoreCache: true);
@@ -288,7 +306,7 @@ Future<void> runWebServiceWorkerTest({
     // Non-caching server
     //////////////////////////////////////////////////////
     print('No cache: test first page load');
-    await _rebuildApp(version: 3, testType: testType);
+    await _rebuildApp(version: 3, testType: testType, target: _target);
     await startAppServer(cacheControl: 'max-age=0');
     await waitForAppToLoad(<String, int>{
       'CLOSE': 1,
@@ -304,6 +322,7 @@ Future<void> runWebServiceWorkerTest({
       'assets/FontManifest.json': 2,
       'flutter_service_worker.js': 1,
       'assets/AssetManifest.json': 1,
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
       'CLOSE': 1,
       // In headless mode Chrome does not load 'manifest.json' and 'favicon.ico'.
       if (!headless)
@@ -329,6 +348,7 @@ Future<void> runWebServiceWorkerTest({
       if (shouldExpectFlutterJs)
         'flutter.js': 1,
       'flutter_service_worker.js': 1,
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
       'CLOSE': 1,
       if (!headless)
         'manifest.json': 1,
@@ -337,7 +357,7 @@ Future<void> runWebServiceWorkerTest({
     reportedVersion = null;
 
     print('No cache: test page reload after rebuild');
-    await _rebuildApp(version: 4, testType: testType);
+    await _rebuildApp(version: 4, testType: testType, target: _target);
 
     // TODO(yjbanov): when running Chrome with DevTools protocol, for some
     // reason a hard refresh is still required. This works without a hard
@@ -357,6 +377,7 @@ Future<void> runWebServiceWorkerTest({
       'main.dart.js': 2,
       'assets/AssetManifest.json': 1,
       'assets/FontManifest.json': 2,
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
       'CLOSE': 1,
       if (!headless)
         ...<String, int>{
@@ -382,3 +403,162 @@ Future<void> runWebServiceWorkerTest({
 
   print('END runWebServiceWorkerTest(headless: $headless, testType: $testType)\n');
 }
+
+Future<void> runWebServiceWorkerTestWithCachingResources({
+  required bool headless,
+  required ServiceWorkerTestType testType
+}) async {
+  final Map<String, int> requestedPathCounts = <String, int>{};
+  void expectRequestCounts(Map<String, int> expectedCounts) =>
+      _expectRequestCounts(expectedCounts, requestedPathCounts);
+
+  AppServer? server;
+  Future<void> waitForAppToLoad(Map<String, int> waitForCounts) async =>
+      _waitForAppToLoad(waitForCounts, requestedPathCounts, server);
+
+  Future<void> startAppServer({
+    required String cacheControl,
+  }) async {
+    final int serverPort = await findAvailablePort();
+    final int browserDebugPort = await findAvailablePort();
+    server = await AppServer.start(
+      headless: headless,
+      cacheControl: cacheControl,
+      // TODO(yjbanov): use a better port disambiguation strategy than trying
+      //                to guess what ports other tests use.
+      appUrl: 'http://localhost:$serverPort/index.html',
+      serverPort: serverPort,
+      browserDebugPort: browserDebugPort,
+      appDirectory: _appBuildDirectory,
+      additionalRequestHandlers: <Handler>[
+            (Request request) {
+          final String requestedPath = request.url.path;
+          requestedPathCounts.putIfAbsent(requestedPath, () => 0);
+          requestedPathCounts[requestedPath] = requestedPathCounts[requestedPath]! + 1;
+          if (requestedPath == 'assets/fonts/MaterialIcons-Regular.otf') {
+            return Response.internalServerError();
+          }
+          return Response.notFound('');
+        },
+      ],
+    );
+  }
+
+  // Preserve old index.html as index_og.html so we can restore it later for other tests
+  await runCommand(
+    'mv',
+    <String>[
+      'index.html',
+      'index_og.html',
+    ],
+    workingDirectory: _testAppWebDirectory,
+  );
+
+  final bool shouldExpectFlutterJs = testType != ServiceWorkerTestType.withoutFlutterJs;
+
+  print('BEGIN runWebServiceWorkerTestWithCachingResources(headless: $headless, testType: $testType)\n');
+
+  try {
+    //////////////////////////////////////////////////////
+    // Caching server
+    //////////////////////////////////////////////////////
+    await _rebuildApp(version: 1, testType: testType, target: _targetWithCachedResources);
+
+    print('With cache: test first page load');
+    await startAppServer(cacheControl: 'max-age=3600');
+    await waitForAppToLoad(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+
+    expectRequestCounts(<String, int>{
+      // Even though the server is caching index.html is downloaded twice,
+      // once by the initial page load, and once by the service worker.
+      // Other resources are loaded once only by the service worker.
+      'index.html': 2,
+      if (shouldExpectFlutterJs)
+        'flutter.js': 1,
+      'main.dart.js': 1,
+      'flutter_service_worker.js': 1,
+      'assets/FontManifest.json': 1,
+      'assets/AssetManifest.json': 1,
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      // In headless mode Chrome does not load 'manifest.json' and 'favicon.ico'.
+      if (!headless)
+        ...<String, int>{
+          'manifest.json': 1,
+          'favicon.ico': 1,
+        },
+    });
+
+    print('With cache: test first page reload');
+    await server!.chrome.reloadPage();
+    await waitForAppToLoad(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+    expectRequestCounts(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+
+    print('With cache: test second page reload');
+    await server!.chrome.reloadPage();
+    await waitForAppToLoad(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+    expectRequestCounts(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+
+    print('With cache: test third page reload');
+    await server!.chrome.reloadPage();
+    await waitForAppToLoad(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+    expectRequestCounts(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+
+    print('With cache: test page reload after rebuild');
+    await _rebuildApp(version: 1, testType: testType, target: _targetWithCachedResources);
+
+    // Since we're caching, we need to ignore cache when reloading the page.
+    await server!.chrome.reloadPage(ignoreCache: true);
+    await waitForAppToLoad(<String, int>{
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      'flutter_service_worker.js': 1,
+    });
+    expectRequestCounts(<String, int>{
+      'index.html': 2,
+      if (shouldExpectFlutterJs)
+        'flutter.js': 1,
+      'main.dart.js': 1,
+      'flutter_service_worker.js': 2,
+      'assets/FontManifest.json': 1,
+      'assets/AssetManifest.json': 1,
+      'assets/fonts/MaterialIcons-Regular.otf': 1,
+      // In headless mode Chrome does not load 'manifest.json' and 'favicon.ico'.
+      if (!headless)
+        ...<String, int>{
+          'favicon.ico': 1,
+        },
+    });
+  } finally {
+    await runCommand(
+      'mv',
+      <String>[
+        'index_og.html',
+        'index.html',
+      ],
+      workingDirectory: _testAppWebDirectory,
+    );
+    await server?.stop();
+  }
+
+  print('END runWebServiceWorkerTestWithCachingResources(headless: $headless, testType: $testType)\n');
+}

dev/bots/test.dart

@@ -1083,6 +1083,9 @@ Future<void> _runWebLongRunningTests() async {
     () => runWebServiceWorkerTest(headless: true, testType: ServiceWorkerTestType.withoutFlutterJs),
     () => runWebServiceWorkerTest(headless: true, testType: ServiceWorkerTestType.withFlutterJs),
     () => runWebServiceWorkerTest(headless: true, testType: ServiceWorkerTestType.withFlutterJsShort),
+    () => runWebServiceWorkerTestWithCachingResources(headless: true, testType: ServiceWorkerTestType.withoutFlutterJs),
+    () => runWebServiceWorkerTestWithCachingResources(headless: true, testType: ServiceWorkerTestType.withFlutterJs),
+    () => runWebServiceWorkerTestWithCachingResources(headless: true, testType: ServiceWorkerTestType.withFlutterJsShort),
     () => _runWebStackTraceTest('profile', 'lib/stack_trace.dart'),
     () => _runWebStackTraceTest('release', 'lib/stack_trace.dart'),
     () => _runWebStackTraceTest('profile', 'lib/framework_stack_trace.dart'),

dev/integration_tests/web/lib/service_worker_test_cached_resources.dart

@@ -0,0 +1,18 @@
+// Copyright 2014 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/material.dart';
+
+Future<void> main() async {
+  runApp(Scaffold(
+    body: Center(
+      child: Column(
+        children: const <Widget>[
+          Icon(Icons.ac_unit),
+          Text('Hello, World', textDirection: TextDirection.ltr),
+        ],
+      ),
+    ),
+  ));
+}

dev/integration_tests/web/pubspec.yaml

@@ -8,6 +8,7 @@ flutter:
   assets:
     - lib/a.dart
     - lib/b.dart
+  uses-material-design: true
 
 dependencies:
   flutter:

packages/flutter_tools/lib/src/web/file_generators/flutter_service_worker_js.dart

@@ -134,9 +134,11 @@ self.addEventListener("fetch", (event) => {
     .then((cache) =>  {
       return cache.match(event.request).then((response) => {
         // Either respond with the cached resource, or perform a fetch and
-        // lazily populate the cache.
+        // lazily populate the cache only if the resource was successfully fetched.
         return response || fetch(event.request).then((response) => {
-          cache.put(event.request, response.clone());
+          if (response && Boolean(response.ok)) {
+            cache.put(event.request, response.clone());
+          }
           return response;
         });
       })