Validate generated MHTML in mhtml_generation_browsertest.cc tests.

third_party/WebKit/Source/core/loader/DocumentLoader.cpp

Index: third_party/WebKit/Source/core/loader/DocumentLoader.cpp
diff --git a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
index 839d841231ae05828c41d5949e949cf0dd66a349..a4e17c2f4e636ba642408d5af3d711ad8d0fd155 100644
--- a/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
+++ b/third_party/WebKit/Source/core/loader/DocumentLoader.cpp
@@ -442,6 +442,13 @@ void DocumentLoader::FinishedLoading(double finish_time) {
     // the Document.
     if (!writer_)
       CommitData(0, 0);

[carlosk, 2017/05/05 23:46:27]

IIUC the local frame is only initialized when data is first committed. As for
empty documents (precisely the case for invalid MHTML) this only happens right
above I chose to have the console message reported here in FinishedLoading
instead of MaybeCreateArchive where I initially thought I would add it.

[Łukasz Anforowicz, 2017/05/06 00:14:48]

Thank you for the comment above.  Unfortunately, I still fail to understand why
the console error is reported here and not in MaybeCreateArchive (in the
|!main_resource| case).  It seems wrong (and I think the current CL does this)
to report the console error when there is a writer (i.e. we have a non-empty
document?) which I think (?) would happen when returning false from
MaybeCreateArchive in |!frame_| case.

[carlosk, 2017/05/16 20:56:08]

I was confused by all the checks for |frame_| not being null and I need it to be
able to log. I assumed the CommitData call above would take care of the writer
and the frame. Upon further looking into the code I now see the frame is
supposed to be passed in upon initialization.

Note that the final outcome in case of an invalid MHTML is an empty document so
that CommitData and further |writer_| creation will happen anyway.

But anyway, as explained before, this all moved to
ResourceFetcher::CreateArchive.

+
+    // Log if attempting to load an invalid archive resource.
+    if (IsArchiveMIMEType(response_.MimeType())) {
+      frame_->GetDocument()->AddConsoleMessage(ConsoleMessage::Create(
+          kOtherMessageSource, kErrorMessageLevel,
+          "Malformed multipart archive: " + response_.Url().ElidedString()));

[Łukasz Anforowicz, 2017/05/06 00:14:48]

I see that KURL::ElidedString returns an empty string if the url is shorter than
1024.  I would not have expected (maybe KURL::ElidedString is wrong?), but in
any case, I think the developers would prefer to see a full URL in an error
message.

[Łukasz Anforowicz, 2017/05/06 00:42:47]

Ooops.  I think I misread the code - what I said above is incorrect.
Still - I wanted to check - do you think it is better to report elided string VS
full string?

[carlosk, 2017/05/16 20:56:08]

This is me copy/pasting without fully understanding what I'm doing. The total
lack of documentation on KURL doesn't help either. :/

You are right, it's detrimental to the developer to not see the full URL.
Changed to GetString().

+    }
   }
 
   if (!frame_)