Handle crash when submitting an injected form on an "image page".

Handle crash when submitting an injected form on an "image document page".

Document encoding isn't set in an image document page. When html+javascript are injected into the document page, which has forms that are submitted with values, it looks for the doc encoding type(which isn't set) to encode the form data. The patch fixes it by When encoding isn't set, derive it from default charset.

BUG=226644
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=159667

[Srini, 2013-10-03 15:56:37]

Hi, can you review my patch. It fixes bug 226644. I can rework if any comments
are there. Thanks.

[Srini, 2013-10-10 09:40:55]

On 2013/10/03 15:56:37, Srini wrote:
> Hi, can you review my patch. It fixes bug 226644. I can rework if any comments
> are there. Thanks.

Hi, can you check this patch? If you aren't the right person, can you point me
whom to ask for review? Thanks!

[pdr., 2013-10-10 19:34:22]

I let this fall off my plate--please make noise if you don't get reviews within
24 hours :)

I'm afraid I don't quite understand what this patch does. Can you update the
change description to describe how a change in FormDataBuilder.cpp fixes an
image bug?

https://codereview.chromium.org/25417007/diff/1/LayoutTests/fast/images/image...
File LayoutTests/fast/images/image-page-injected-script-crash.html (right):

https://codereview.chromium.org/25417007/diff/1/LayoutTests/fast/images/image...
LayoutTests/fast/images/image-page-injected-script-crash.html:23: o.innerHTML =
'<div class="modal fade" id="new-pin"><div class="modal-header"><h3>New
Pin</h3></div><form name="testForm" action="" enctype="multipart/form-data"
method="get" id="ajaxform" name="pin_form" class="form-horizontal"><div
class="modal-body"><div id="div_id_url" class="control-group"><label
class="control-label" for="id_url">URL</label><div class="controls"><input
type="text" name="url" id="id_url" /></div></div><div id="div_id_image"
class="control-group"><label class="control-label" for="id_image">or
Upload</label><div class="controls"><input type="file" name="image"
id="id_image" /></div></div><div id="div_id_description"
class="control-group"><label class="control-label"
for="id_description">Description</label><div class="controls"><textarea
id="id_description" rows="10" cols="40"
name="description"></textarea></div></div><div id="div_id_tags"
class="control-group"><label class="control-label"
for="id_tags">Tags</label><div class="controls"><input type="text" name="tags"
id="id_tags" /></div></div></div><div class="modal-footer"><div
class="messageContainer"></div><button id="btnsubmit" type="submit" class="btn
btn-primary">Submit</button><a id="cancel" onclick="removeOverlay()"
data-toggle="modal" class="btn">Cancel</a></div></form></div>';
Is all of this HTML necessary?

[Srini, 2013-10-10 19:51:05]

On 2013/10/10 19:34:22, pdr wrote:
> I let this fall off my plate--please make noise if you don't get reviews
within
> 24 hours :)
> 
> I'm afraid I don't quite understand what this patch does. Can you update the
> change description to describe how a change in FormDataBuilder.cpp fixes an
> image bug?

Its not exactly an Image bug. In an image document, when html form is injected,
the document encoding isn't set. When this isn't set and the injected form is
submitted, it tries to use the encoding to encode the form data elements and
crash. So this patch fixes that if the encoding isn't set, derive it from
charset. 

> 
>
https://codereview.chromium.org/25417007/diff/1/LayoutTests/fast/images/image...
> File LayoutTests/fast/images/image-page-injected-script-crash.html (right):
> 
>
https://codereview.chromium.org/25417007/diff/1/LayoutTests/fast/images/image...
> LayoutTests/fast/images/image-page-injected-script-crash.html:23: o.innerHTML
=
> '<div class="modal fade" id="new-pin"><div class="modal-header"><h3>New
> Pin</h3></div><form name="testForm" action="" enctype="multipart/form-data"
> method="get" id="ajaxform" name="pin_form" class="form-horizontal"><div
> class="modal-body"><div id="div_id_url" class="control-group"><label
> class="control-label" for="id_url">URL</label><div class="controls"><input
> type="text" name="url" id="id_url" /></div></div><div id="div_id_image"
> class="control-group"><label class="control-label" for="id_image">or
> Upload</label><div class="controls"><input type="file" name="image"
> id="id_image" /></div></div><div id="div_id_description"
> class="control-group"><label class="control-label"
> for="id_description">Description</label><div class="controls"><textarea
> id="id_description" rows="10" cols="40"
> name="description"></textarea></div></div><div id="div_id_tags"
> class="control-group"><label class="control-label"
> for="id_tags">Tags</label><div class="controls"><input type="text" name="tags"
> id="id_tags" /></div></div></div><div class="modal-footer"><div
> class="messageContainer"></div><button id="btnsubmit" type="submit" class="btn
> btn-primary">Submit</button><a id="cancel" onclick="removeOverlay()"
> data-toggle="modal" class="btn">Cancel</a></div></form></div>';
> Is all of this HTML necessary?

Sorry. It is kind of good to have. I derived this from the original bug report.
Even when run manually, it is a bit descriptive to type and test.

[pdr., 2013-10-10 23:15:24]

Thanks for updating the description.

LGTM, this sounds reasonable to me. Please do clean up the test before
submitting.

https://codereview.chromium.org/25417007/diff/1/LayoutTests/fast/images/image...
File LayoutTests/fast/images/image-page-injected-script-crash.html (right):

https://codereview.chromium.org/25417007/diff/1/LayoutTests/fast/images/image...
LayoutTests/fast/images/image-page-injected-script-crash.html:23: o.innerHTML =
'<div class="modal fade" id="new-pin"><div class="modal-header"><h3>New
Pin</h3></div><form name="testForm" action="" enctype="multipart/form-data"
method="get" id="ajaxform" name="pin_form" class="form-horizontal"><div
class="modal-body"><div id="div_id_url" class="control-group"><label
class="control-label" for="id_url">URL</label><div class="controls"><input
type="text" name="url" id="id_url" /></div></div><div id="div_id_image"
class="control-group"><label class="control-label" for="id_image">or
Upload</label><div class="controls"><input type="file" name="image"
id="id_image" /></div></div><div id="div_id_description"
class="control-group"><label class="control-label"
for="id_description">Description</label><div class="controls"><textarea
id="id_description" rows="10" cols="40"
name="description"></textarea></div></div><div id="div_id_tags"
class="control-group"><label class="control-label"
for="id_tags">Tags</label><div class="controls"><input type="text" name="tags"
id="id_tags" /></div></div></div><div class="modal-footer"><div
class="messageContainer"></div><button id="btnsubmit" type="submit" class="btn
btn-primary">Submit</button><a id="cancel" onclick="removeOverlay()"
data-toggle="modal" class="btn">Cancel</a></div></form></div>';

Please remove the unnecessary style data here (e.g., class="modal fade". etc).
You may even consider splitting this up into multiple lines, but that's
optional.

The reason I ask for this is: if this test fails sometime in the future, it's
not easy to see what it does. By simplifying this part of the test, future
people in this code will have an easier time figuring out what's going on
without having to look up this review for context.

[pdr., 2013-10-10 23:15:25]

Thanks for updating the description.

LGTM, this sounds reasonable to me. Please do clean up the test before
submitting.

[Srini, 2013-10-11 14:03:02]

On 2013/10/10 23:15:25, pdr wrote:
> Thanks for updating the description.
> 
> LGTM, this sounds reasonable to me. Please do clean up the test before
> submitting.

Thx pdr. I will clean up the test and submit it.

[commit-bot: I haz the power, 2013-10-15 10:23:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/srinivasa.ragavan.venkateswaran@intel....

[commit-bot: I haz the power, 2013-10-15 12:37:57]

Change committed as 159667