Revert "Revert "Fix extraneous call to ContentVerifyJob::DispatchFailureCallback.""

This reverts commit 2181b01c.

Reason for revert: The offending test
(ContentVerifierTest.FailOnRead) has been taken care of in
CL: https://chromium-review.googlesource.com/c/chromium/src/+/888327

Original change's description:
> Revert "Fix extraneous call to ContentVerifyJob::DispatchFailureCallback."
>
> This reverts commit 6e548ee1.
>
> Reason for revert: DCHECK at content_verify_job.cc:192 occasionally fails (see crbug.com/806586).
>
> Original change's description:
> > Fix extraneous call to ContentVerifyJob::DispatchFailureCallback.
> >
> > If ContentVerifyJob receives hashes (ContentVerifyJob::OnHashesReady)
> > after the content is read (ContentVerifyJob::DoneReading), then it
> > was possible for DispatchFailureCallback to be called more than once.
> > This causes a DCHECK failure in DispatchFailureCallback
> > (DCHECK(!failed_)).
> >
> > Bail out early in OnHashesReady if call to ContentVerifyJob::BytesRead
> > leads to a ContentVerifyJob failure.
> >
> > Note that this isn't harmful in Release builds as
> > DispatchFailureCallback tests ContentVerifyJob::failure_callback_
> > before calling the callback, which is reset once it is called.
> > Nevertheless, this is incorrect behavior and this CL fixes that.
> >
> >
> > Bug: 804630
> > Test: See bug description for repro steps.
> > Change-Id: Icf3a5c8b4c0d01cb20e02de14b11aca4aeff03e5
> > Reviewed-on: https://chromium-review.googlesource.com/879961
> > Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
> > Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#531755}
>
> TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org
>
> # Not skipping CQ checks because original CL landed > 1 day ago.
>
> Bug: 804630,806586
> Change-Id: Ia5c67f60e95ddb607cb364c745f31dde465d3c4f
> Reviewed-on: https://chromium-review.googlesource.com/893258
> Reviewed-by: vitaliii <vitaliii@chromium.org>
> Commit-Queue: vitaliii <vitaliii@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#532854}

TBR=lazyboy@chromium.org,rdevlin.cronin@chromium.org,vitaliii@chromium.org

Change-Id: I82988d836f49033e1d618125e06f0d45d1124071
Bug: 804630, 806586
Reviewed-on: https://chromium-review.googlesource.com/894824
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533209}

extensions/browser/content_verifier/content_hash.h

@@ -12,6 +12,7 @@
 #include "extensions/browser/computed_hashes.h"
 #include "extensions/browser/content_verifier_delegate.h"
 #include "extensions/browser/verified_contents.h"
+#include "extensions/common/constants.h"
 #include "extensions/common/extension_id.h"
 
 namespace extensions {
@@ -137,7 +138,7 @@ class ContentHash {
   // The block size to use for hashing.
   // TODO(asargent) - use the value from verified_contents.json for each
   // file, instead of using a constant.
-  int block_size_ = 4096;
+  int block_size_ = extension_misc::kContentVerificationDefaultBlockSize;
 
   DISALLOW_COPY_AND_ASSIGN(ContentHash);
 };

extensions/browser/content_verify_job.cc

@@ -128,6 +128,7 @@ void ContentVerifyJob::DoneReading() {
 }
 
 bool ContentVerifyJob::FinishBlock() {
+  DCHECK(!failed_);
   if (current_hash_byte_count_ == 0) {
     if (!done_reading_ ||
         // If we have checked all blocks already, then nothing else to do here.
@@ -180,11 +181,15 @@ void ContentVerifyJob::OnHashesReady(bool success) {
     return;
   }
 
+  DCHECK(!failed_);
+
   hashes_ready_ = true;
   if (!queue_.empty()) {
     std::string tmp;
     queue_.swap(tmp);
     BytesRead(tmp.size(), base::string_as_array(&tmp));
+    if (failed_)
+      return;
   }
   if (done_reading_) {
     ScopedElapsedTimer timer(&time_spent_);

extensions/browser/content_verify_job_unittest.cc

@@ -207,6 +207,14 @@ TEST_F(ContentVerifyJobUnittest, ContentMismatch) {
   RunContentMismatchTest("console.log('modified');");
 }
 
+// Similar to ContentMismatch, but uses a file size > 4k.
+// Regression test for https://crbug.com/804630.
+TEST_F(ContentVerifyJobUnittest, ContentMismatchWithLargeFile) {
+  std::string content_larger_than_block_size(
+      extension_misc::kContentVerificationDefaultBlockSize + 1, ';');
+  RunContentMismatchTest(content_larger_than_block_size);
+}
+
 // Tests that extension resources that are originally 0 byte behave correctly
 // with content verification.
 TEST_F(ContentVerifyJobUnittest, LegitimateZeroByteFile) {

extensions/common/constants.cc

@@ -119,4 +119,6 @@ const char* const kHangoutsExtensionIds[6] = {
 const char kPolicyBlockedScripting[] =
     "This page cannot be scripted due to an ExtensionsSettings policy.";
 
+const int kContentVerificationDefaultBlockSize = 4096;
+
 }  // namespace extension_misc

extensions/common/constants.h

@@ -237,6 +237,9 @@ extern const char* const kHangoutsExtensionIds[6];
 // Error message when enterprise policy blocks scripting of webpage.
 extern const char kPolicyBlockedScripting[];
 
+// The default block size for hashing used in content verification.
+extern const int kContentVerificationDefaultBlockSize;
+
 }  // namespace extension_misc
 
 #endif  // EXTENSIONS_COMMON_CONSTANTS_H_