MHTML generation: safely serialize image documents

MHTML generation: safely serialize image documents

Downstream code expects nonzero resources to be discovered during
FrameSerializer::serializeFrame. However, this can happen in some cases
when processing image documents.

BUG=678893
Review-Url: https://codereview.chromium.org/2613253002
Cr-Commit-Position: refs/heads/master@{#442058}
Committed: https://chromium.googlesource.com/chromium/src/+/18a3a8709bd95e5a90aa5892962790688e3620a5

[Charlie Harrison, 2017-01-06 15:57:47]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 15:58:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-01-06 16:12:34]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-06 16:12:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-01-06 16:14:05]

csharrison@chromium.org changed reviewers:
+ carlosk@chromium.org, dcheng@chromium.org

[Charlie Harrison, 2017-01-06 16:14:05]

dcheng, carlosk PTAL?

[dcheng, 2017-01-06 16:31:57]

LGTM

[Charlie Harrison, 2017-01-06 16:32:48]

The CQ bit was unchecked by csharrison@chromium.org

[Charlie Harrison, 2017-01-06 16:32:48]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2017-01-06 16:33:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 17:55:34]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 17:55:35]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Charlie Harrison, 2017-01-06 17:59:23]

The CQ bit was checked by csharrison@chromium.org

[commit-bot: I haz the power, 2017-01-06 17:59:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-06 21:33:14]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483725563251340,
"parent_rev": "27140585843585d65c219884ff31719fbb61dbaf", "commit_rev":
"18a3a8709bd95e5a90aa5892962790688e3620a5"}

[commit-bot: I haz the power, 2017-01-06 21:33:44]

Description was changed from

==========
MHTML generation: safely serialize image documents

Downstream code expects nonzero resources to be discovered during
FrameSerializer::serializeFrame. However, this can happen in some cases
when processing image documents.

BUG=678893
==========

to

==========
MHTML generation: safely serialize image documents

Downstream code expects nonzero resources to be discovered during
FrameSerializer::serializeFrame. However, this can happen in some cases
when processing image documents.

BUG=678893

Review-Url: https://codereview.chromium.org/2613253002
Cr-Commit-Position: refs/heads/master@{#442058}
Committed:
https://chromium.googlesource.com/chromium/src/+/18a3a8709bd95e5a90aa58929627...
==========

[commit-bot: I haz the power, 2017-01-06 21:33:45]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/18a3a8709bd95e5a90aa58929627...

[carlosk, 2017-01-06 23:26:36]

Thanks for fixing that bug. There is one simple modification still needed
though.

https://codereview.chromium.org/2613253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2613253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:300: return
WebThreadSafeData();
To keep tracing logic consistent this should be moved after the TRACE_EVENT_END1
below.

[Charlie Harrison, 2017-01-07 02:31:25]

https://codereview.chromium.org/2613253002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebFrameSerializer.cpp (right):

https://codereview.chromium.org/2613253002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebFrameSerializer.cpp:300: return
WebThreadSafeData();
On 2017/01/06 23:26:36, carlosk wrote:
> To keep tracing logic consistent this should be moved after the
TRACE_EVENT_END1
> below.

Good catch, will fix this in a follow up.