Validate generated MHTML in mhtml_generation_browsertest.cc tests.

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

 /*
  * Copyright (C) 2006, 2007, 2008 Apple Inc. All rights reserved.
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
@@ skipping 423 matching lines @@
   if (!response_end_time)
     response_end_time = time_of_last_data_received_;
   if (!response_end_time)
     response_end_time = MonotonicallyIncreasingTime();
   GetTiming().SetResponseEnd(response_end_time);
   if (!MaybeCreateArchive()) {
     // If this is an empty document, it will not have actually been created yet.
     // Commit dummy data so that DocumentWriter::begin() gets called and creates
     // 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_)
     return;
 
   application_cache_host_->FinishedLoadingMainResource();
   EndWriting();
   ClearMainResourceHandle();
 }
 
@@ skipping 648 matching lines @@
                      InstallNewDocumentReason::kJavascriptURL,
                      kForceSynchronousParsing, KURL());
   if (!source.IsNull())
     writer_->AppendReplacingData(source);
   EndWriting();
 }
 
 DEFINE_WEAK_IDENTIFIER_MAP(DocumentLoader);
 
 }  // namespace blink