Only set FileReader result on Load

This CL changes the behaviour of FileReader so that the result attribute
 is only set right before the Load event is fired.

The existing web tests that covered this behaviour have their expected
result changed to now be passing (the expectation file was deleted as
all tests now pass).

The CL also removes the kFileReaderResultBeforeCompletion feature which
measured how often the result attribute was accessed before load. This
feature is no longer needed.

Change-Id: Ide34aa7ebe5d4caadc2c40821b1eb7cbbe7e91d7
Bug: 768972
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352610Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Shubham Aggarwal <shuagga@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#803174}

third_party/blink/public/mojom/web_feature/web_feature.mojom

@@ -1576,7 +1576,6 @@ enum WebFeature {
   kFeaturePolicyAllowAttributeDeprecatedSyntax = 2145,
   kSuppressHistoryEntryWithoutUserGesture = 2146,
   kPerformanceServerTiming = 2157,
-  kFileReaderResultBeforeCompletion = 2158,
   kAnimationSetPlaybackRateCompensatorySeek = 2162,
   kDeepCombinatorInStaticProfile = 2163,
   kPseudoShadowInStaticProfile = 2164,

third_party/blink/renderer/core/fileapi/file_reader.cc

@@ -72,7 +72,6 @@ static const base::TimeDelta kProgressNotificationInterval =
 class FileReader::ThrottlingController final
     : public GarbageCollected<FileReader::ThrottlingController>,
       public Supplement<ExecutionContext> {
-
  public:
   static const char kSupplementName[];
 
@@ -359,14 +358,15 @@ void FileReader::abort() {
   Terminate();
 }
 
-void FileReader::result(ScriptState* state,
-                        StringOrArrayBuffer& result_attribute) const {
+void FileReader::result(StringOrArrayBuffer& result_attribute) const {
   if (error_ || !loader_)
     return;
 
-  if (!loader_->HasFinishedLoading()) {
-    UseCounter::Count(ExecutionContext::From(state),
-                      WebFeature::kFileReaderResultBeforeCompletion);
+  // Only set the result after |loader_| has finished loading which means that
+  // FileReader::DidFinishLoading() has also been called. This ensures that the
+  // result is not available until just before the kLoad event is fired.
+  if (!loader_->HasFinishedLoading() || state_ != ReadyState::kDone) {
+    return;
   }
 
   if (read_type_ == FileReaderLoader::kReadAsArrayBuffer)

third_party/blink/renderer/core/fileapi/file_reader.h

@@ -74,7 +74,7 @@ class CORE_EXPORT FileReader final : public EventTargetWithInlineData,
 
   ReadyState getReadyState() const { return state_; }
   DOMException* error() { return error_; }
-  void result(ScriptState*, StringOrArrayBuffer& result_attribute) const;
+  void result(StringOrArrayBuffer& result_attribute) const;
   probe::AsyncTaskId* async_task_id() { return &async_task_id_; }
 
   // ExecutionContextLifecycleObserver

third_party/blink/renderer/core/fileapi/file_reader.idl

@@ -52,7 +52,6 @@
     [ImplementedAs=getReadyState] readonly attribute unsigned short readyState;
 
     // File or Blob data
-    [CallWith=ScriptState]
     readonly attribute (DOMString or ArrayBuffer)? result;
 
     readonly attribute DOMException? error;

third_party/blink/web_tests/external/wpt/FileAPI/reading-data-section/filereader_result-expected.txt

-This is a testharness.js-based test.
-PASS readAsText
-PASS readAsDataURL
-PASS readAsArrayBuffer
-PASS readAsBinaryString
-FAIL result is null during "loadstart" event for readAsText assert_equals: result is null after first read call expected (object) null but got (string) ""
-FAIL result is null during "loadstart" event for readAsDataURL assert_equals: result is null after first read call expected (object) null but got (string) ""
-FAIL result is null during "loadstart" event for readAsArrayBuffer assert_equals: result is null during event expected null but got object "[object ArrayBuffer]"
-FAIL result is null during "loadstart" event for readAsBinaryString assert_equals: result is null after first read call expected (object) null but got (string) ""
-FAIL result is null during "progress" event for readAsText assert_equals: result is null after first read call expected (object) null but got (string) ""
-FAIL result is null during "progress" event for readAsDataURL assert_equals: result is null after first read call expected (object) null but got (string) ""
-FAIL result is null during "progress" event for readAsArrayBuffer assert_equals: result is null during event expected null but got object "[object ArrayBuffer]"
-FAIL result is null during "progress" event for readAsBinaryString assert_equals: result is null after first read call expected (object) null but got (string) ""
-Harness: the test ran to completion.
-

third_party/blink/web_tests/fast/files/read-blob-as-array-buffer.html

@@ -21,10 +21,7 @@ test.step(function() {
         assert_equals(reader.readyState, reader.LOADING,
                       "reader.readyState");
         assert_equals(reader.error, null, "reader.error");
-        assert_not_equals(reader.result, null, "reader.result");
-        assert_true(reader.result.byteLength >= 0 &&
-                    reader.result.byteLength <= 10,
-                    "reader.result.byteLength between 0 and 10 inclusive");
+        assert_equals(reader.result, null, "reader.result");
     });
     reader.onabort = test.step_func(function() {
         assert_unreached("onabort invoked on reader");

third_party/blink/web_tests/fast/files/read-file-async-expected.txt

@@ -193,7 +193,7 @@ result size: 5
 result: Hello
 Received loadend event
 readyState after recalling read method: 1
-result after recalling read method: 
+result after recalling read method: null
 error after recalling read method: null
 Received loadstart event
 readyState: 1
@@ -210,7 +210,7 @@ error name: NotFoundError
 result: null
 Received loadend event
 readyState after recalling read method: 1
-result after recalling read method: 
+result after recalling read method: null
 error after recalling read method: null
 Received loadstart event
 readyState: 1

third_party/blink/web_tests/fast/files/workers/worker-read-file-async-expected.txt

@@ -195,7 +195,7 @@ result size: 5
 result: Hello
 Received loadend event
 readyState after recalling read method: 1
-result after recalling read method: 
+result after recalling read method: null
 error after recalling read method: null
 Received loadstart event
 readyState: 1
@@ -212,7 +212,7 @@ error name: NotFoundError
 result: null
 Received loadend event
 readyState after recalling read method: 1
-result after recalling read method: 
+result after recalling read method: null
 error after recalling read method: null
 Received loadstart event
 readyState: 1