Archive loading should be case insensitive to mimetype "multipart/related"

Archive loading should be case insensitive to mimetype "multipart/related"
    
Before this patch only archive with lowercase mimetype is loaded.Since mimetype is case insensitive subject
so browser should also load archive whose mimetype contains uppercase character.
    
BUG=None
TEST=LayoutTests/mhtml/mhtml-with-capital-mimetype-loading.mht

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

[samahto, 2015-03-21 08:20:56]

samahto@cisco.com changed reviewers:
+ japhet@chromium.org

[Nate Chapin, 2015-03-23 16:55:55]

On 2015/03/21 08:21:26, samahto wrote:

This code change looks fine. Test?

[samahto, 2015-03-31 16:29:56]

On 2015/03/23 16:55:55, Nate Chapin wrote:
> On 2015/03/21 08:21:26, samahto wrote:
> 
> This code change looks fine. Test?

Hi, could you guide me adding test cases for this. I am finding it difficult to
hook it with Layouttests.
May be in this case test should be ignored.

[Nate Chapin, 2015-03-31 17:50:40]

On 2015/03/31 16:29:56, samahto wrote:
> On 2015/03/23 16:55:55, Nate Chapin wrote:
> > On 2015/03/21 08:21:26, samahto wrote:
> > 
> > This code change looks fine. Test?
> 
> Hi, could you guide me adding test cases for this. I am finding it difficult
to
> hook it with Layouttests.
> May be in this case test should be ignored.

I don't know for sure whether this will work, but I would try taking one of the
.mht files in LayoutTests/mhtml, changing the case of the "multipart/related" in
that file, and see if it fails without this change but passes with it.

[samahto, 2015-04-05 10:14:57]

On 2015/03/31 17:50:40, Nate Chapin wrote:
> On 2015/03/31 16:29:56, samahto wrote:
> > On 2015/03/23 16:55:55, Nate Chapin wrote:
> > > On 2015/03/21 08:21:26, samahto wrote:
> > > 
> > > This code change looks fine. Test?
> > 
> > Hi, could you guide me adding test cases for this. I am finding it difficult
> to
> > hook it with Layouttests.
> > May be in this case test should be ignored.
> 
> I don't know for sure whether this will work, but I would try taking one of
the
> .mht files in LayoutTests/mhtml, changing the case of the "multipart/related"
in
> that file, and see if it fails without this change but passes with it.

updated the testcase.
Before this archive loading was not possible for capital letter Mimetype.
Now it loads fine.
Please review

[samahto, 2015-04-05 18:19:13]

On 2015/04/05 10:14:57, samahto wrote:
> On 2015/03/31 17:50:40, Nate Chapin wrote:
> > On 2015/03/31 16:29:56, samahto wrote:
> > > On 2015/03/23 16:55:55, Nate Chapin wrote:
> > > > On 2015/03/21 08:21:26, samahto wrote:
> > > > 
> > > > This code change looks fine. Test?
> > > 
> > > Hi, could you guide me adding test cases for this. I am finding it
difficult
> > to
> > > hook it with Layouttests.
> > > May be in this case test should be ignored.
> > 
> > I don't know for sure whether this will work, but I would try taking one of
> the
> > .mht files in LayoutTests/mhtml, changing the case of the
"multipart/related"
> in
> > that file, and see if it fails without this change but passes with it.
> 
> updated the testcase.
> Before this archive loading was not possible for capital letter Mimetype.
> Now it loads fine.
> Please review
I discovered some issue with patch so please dont review till next call

[samahto, 2015-04-06 02:50:44]

I have updated the patch. Now patch is ready for review.

[sof, 2015-04-06 05:27:36]

https://codereview.chromium.org/1027623003/diff/40001/Source/platform/mhtml/M...
File Source/platform/mhtml/MHTMLParser.cpp (right):

https://codereview.chromium.org/1027623003/diff/40001/Source/platform/mhtml/M...
Source/platform/mhtml/MHTMLParser.cpp:68: bool isMultipart() const { return
m_contentType.lower().startsWith("multipart/"); }
startsWith() takes an optional 2nd arg controlling case sensitivity,

  <...>.startsWith("multipart/", TextCaseInsensitive);

[samahto, 2015-04-07 10:18:54]

On 2015/04/06 05:27:36, sof wrote:
>
https://codereview.chromium.org/1027623003/diff/40001/Source/platform/mhtml/M...
> File Source/platform/mhtml/MHTMLParser.cpp (right):
> 
>
https://codereview.chromium.org/1027623003/diff/40001/Source/platform/mhtml/M...
> Source/platform/mhtml/MHTMLParser.cpp:68: bool isMultipart() const { return
> m_contentType.lower().startsWith("multipart/"); }
> startsWith() takes an optional 2nd arg controlling case sensitivity,
> 
>   <...>.startsWith("multipart/", TextCaseInsensitive);

Thanks for pointing out. I updated the patch as per comments.
Please review it.

[samahto, 2015-04-07 10:20:17]

samahto@cisco.com changed reviewers:
+ sigbjorn@opera.com

[sof, 2015-04-07 13:00:42]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com
- sigbjorn@opera.com

[sof, 2015-04-07 13:00:43]

lgtm, thanks for fixing this.

You'll need approval for the Source/platform/ change though ( Cc:'ed mkwst who
might have the time for a quick peek? )

[Mike West, 2015-04-07 13:27:25]

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

[Mike West, 2015-04-07 13:27:26]

LGTM.

[samahto, 2015-04-07 14:43:46]

The CQ bit was checked by samahto@cisco.com

[commit-bot: I haz the power, 2015-04-07 14:43:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/60001

[samahto, 2015-04-07 15:01:58]

The CQ bit was unchecked by samahto@cisco.com

[samahto, 2015-04-07 15:04:43]

The CQ bit was checked by samahto@cisco.com

[commit-bot: I haz the power, 2015-04-07 15:04:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/60001

[samahto, 2015-04-07 15:06:08]

samahto@cisco.com changed reviewers:
- japhet@chromium.org, mkwst@chromium.org, sigbjornf@opera.com

[samahto, 2015-04-07 15:06:16]

The CQ bit was unchecked by samahto@cisco.com

[samahto, 2015-04-07 15:06:50]

The CQ bit was checked by samahto@cisco.com

[samahto, 2015-04-07 15:07:02]

The CQ bit was unchecked by samahto@cisco.com

[samahto, 2015-04-07 15:08:19]

The CQ bit was checked by samahto@cisco.com

[commit-bot: I haz the power, 2015-04-07 15:08:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/60001

[commit-bot: I haz the power, 2015-04-07 18:37:38]

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

[commit-bot: I haz the power, 2015-04-07 18:37:40]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)

[samahto, 2015-04-08 02:31:27]

The CQ bit was checked by samahto@cisco.com

[commit-bot: I haz the power, 2015-04-08 02:31:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/60001

[commit-bot: I haz the power, 2015-04-08 05:47:02]

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

[commit-bot: I haz the power, 2015-04-08 05:47:03]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/5...)

[samahto, 2015-04-08 19:35:38]

The CQ bit was checked by samahto@cisco.com

[commit-bot: I haz the power, 2015-04-08 19:36:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/60001

[commit-bot: I haz the power, 2015-04-08 23:40:41]

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

[commit-bot: I haz the power, 2015-04-08 23:40:42]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/51022)

[samahto, 2015-04-11 11:29:03]

The CQ bit was checked by samahto@cisco.com

[samahto, 2015-04-11 11:29:04]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1027623003/#ps80001
(title: "Minor changes")

[commit-bot: I haz the power, 2015-04-11 11:29:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/80001

[commit-bot: I haz the power, 2015-04-11 15:47:18]

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

[commit-bot: I haz the power, 2015-04-11 15:47:20]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/51460)

[samahto, 2015-04-13 07:20:30]

commit-bot is failing because review board is replacing CRLF to LF.I dont't know
how to overcome this.please guide me.
This issue has been discussed long before
here(https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/WzBiNHj53gs)
This is old discussion trying to fix the same. but I am not sure why I am facing
this. Could anyone  help me in landing this.

[samahto, 2015-04-13 09:59:19]

On 2015/04/07 13:00:43, sof wrote:
> lgtm, thanks for fixing this.
> 
> You'll need approval for the Source/platform/ change though ( Cc:'ed mkwst who
> might have the time for a quick peek? )

I am facing issue(tests are failing) in landing this(reviewboard is converting
CRLF to LF in mhtml file). could you help me in that.

[sof, 2015-04-23 07:57:54]

On 2015/04/13 09:59:19, samahto wrote:
> On 2015/04/07 13:00:43, sof wrote:
> > lgtm, thanks for fixing this.
> > 
> > You'll need approval for the Source/platform/ change though ( Cc:'ed mkwst
who
> > might have the time for a quick peek? )
> 
> I am facing issue(tests are failing) in landing this(reviewboard is converting
> CRLF to LF in mhtml file). could you help me in that.

Sorry, for some reason rietveld didn't show this review as updated on my list of
incoming reviews after #44.

Anyway, as mentioned via IRC, treating .mht's as binary files has worked for
others previously.

[samahto, 2015-04-26 12:18:50]

The CQ bit was checked by samahto@cisco.com

[samahto, 2015-04-26 12:18:50]

The patchset sent to the CQ was uploaded after l-g-t-m from sigbjornf@opera.com,
mkwst@chromium.org
Link to the patchset: https://codereview.chromium.org/1027623003/#ps100001
(title: ".mht uploaded as binary")

[commit-bot: I haz the power, 2015-04-26 12:18:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1027623003/100001

[commit-bot: I haz the power, 2015-04-26 13:33:47]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194450