ParsedContentType refactoring

ParsedContentType refactoring

This is a preliminary refactoring for fixing compatibility issues of
ParsedContentType.

BUG=689731
Review-Url: https://codereview.chromium.org/2704153002
Cr-Commit-Position: refs/heads/master@{#452045}
Committed: https://chromium.googlesource.com/chromium/src/+/d4c7568248450ac03286a4af6fe46380b582ed68

[yhirano, 2017-02-20 07:42:16]

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

[commit-bot: I haz the power, 2017-02-20 07:42:33]

Dry run: 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-02-20 08:46:33]

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

[commit-bot: I haz the power, 2017-02-20 08:46:34]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yhirano, 2017-02-20 10:27:26]

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

[yhirano, 2017-02-20 10:27:41]

yhirano@chromium.org changed reviewers:
+ kinuko@chromium.org

[commit-bot: I haz the power, 2017-02-20 10:27:42]

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

[yhirano, 2017-02-20 11:03:56]

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

[commit-bot: I haz the power, 2017-02-20 11:04:18]

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

[kinuko, 2017-02-20 11:56:12]

lgtm

[commit-bot: I haz the power, 2017-02-20 12:49:30]

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

[commit-bot: I haz the power, 2017-02-20 12:49:31]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tyoshino-do-not-use, 2017-02-20 14:48:46]

tyoshino@google.com changed reviewers:
+ tyoshino@google.com

[tyoshino-do-not-use, 2017-02-20 14:48:47]

lgtm

https://codereview.chromium.org/2704153002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ParsedContentType.cpp (right):

https://codereview.chromium.org/2704153002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ParsedContentType.cpp:81:
parse(contentType.stripWhiteSpace());
How about saving the result of parse() in a member indicating validness of the
ParsedContentType like GURL?

The users would call ParsedContentType(contentType).isValid() instead of
ParsedContentType::isValid(contentType). It wouldn't break
MIMEHeader::parseHeader() since retrieveKeyValuePairs() strips \r and \n.

[tyoshino-do-not-use, 2017-02-20 14:50:31]

https://codereview.chromium.org/2704153002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ParsedContentType.cpp (right):

https://codereview.chromium.org/2704153002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ParsedContentType.cpp:81:
parse(contentType.stripWhiteSpace());
On 2017/02/20 14:48:46, tyoshino-do-not-use wrote:
> How about saving the result of parse() in a member indicating validness of the
> ParsedContentType like GURL?
> 
> The users would call ParsedContentType(contentType).isValid() instead of
> ParsedContentType::isValid(contentType). It wouldn't break
> MIMEHeader::parseHeader() since retrieveKeyValuePairs() strips \r and \n.

Sorry. I forgot to insert break in the last paragraph and explain regarding what
concern I wrote the last sentence. I meant that skipping parse() in case
contentType contains \r or \n doesn't break MIMEHeader::parseHeader().

[yhirano, 2017-02-21 04:39:17]

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

[commit-bot: I haz the power, 2017-02-21 04:39:33]

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

[yhirano, 2017-02-21 04:40:30]

https://codereview.chromium.org/2704153002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/network/ParsedContentType.cpp (right):

https://codereview.chromium.org/2704153002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/network/ParsedContentType.cpp:81:
parse(contentType.stripWhiteSpace());
On 2017/02/20 14:50:31, tyoshino-do-not-use wrote:
> On 2017/02/20 14:48:46, tyoshino-do-not-use wrote:
> > How about saving the result of parse() in a member indicating validness of
the
> > ParsedContentType like GURL?
> > 
> > The users would call ParsedContentType(contentType).isValid() instead of
> > ParsedContentType::isValid(contentType). It wouldn't break
> > MIMEHeader::parseHeader() since retrieveKeyValuePairs() strips \r and \n.
> 
> Sorry. I forgot to insert break in the last paragraph and explain regarding
what
> concern I wrote the last sentence. I meant that skipping parse() in case
> contentType contains \r or \n doesn't break MIMEHeader::parseHeader().

Done.

[tyoshino (SeeGerritForStatus), 2017-02-21 05:18:28]

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

[tyoshino (SeeGerritForStatus), 2017-02-21 05:18:28]

lgtm

[yhirano, 2017-02-21 05:23:53]

The CQ bit was unchecked by yhirano@chromium.org

[yhirano, 2017-02-21 05:23:54]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2017-02-21 05:23:54]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
tyoshino@google.com
Link to the patchset: https://codereview.chromium.org/2704153002/#ps60001
(title: "fix")

[commit-bot: I haz the power, 2017-02-21 05:24:00]

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-02-21 05:30:14]

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

[commit-bot: I haz the power, 2017-02-21 05:30:15]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[yhirano, 2017-02-21 06:29:45]

yhirano@chromium.org changed reviewers:
+ jrummell@chromium.org

[yhirano, 2017-02-21 06:29:46]

+jrummell@chromium.org for OWNER review.

[haraken, 2017-02-21 06:30:27]

owner LGTM

[yhirano, 2017-02-21 06:31:53]

Thank you!

[yhirano, 2017-02-21 06:31:58]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2017-02-21 06:32:35]

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

[yhirano, 2017-02-21 06:46:59]

Description was changed from

==========
ParsedContentType refactoring

This is a preliminary refactoring for fix compatibility issues of
ParsedContentType.

BUG=689731
==========

to

==========
ParsedContentType refactoring

This is a preliminary refactoring for fixing compatibility issues of
ParsedContentType.

BUG=689731
==========

[commit-bot: I haz the power, 2017-02-21 09:32:34]

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

[commit-bot: I haz the power, 2017-02-21 09:32:36]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[jrummell, 2017-02-21 22:58:48]

media LGTM

[yhirano, 2017-02-22 00:25:45]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2017-02-22 00:27:37]

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-02-22 02:30:51]

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

[commit-bot: I haz the power, 2017-02-22 02:30:53]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[yhirano, 2017-02-22 03:24:17]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2017-02-22 03:25:36]

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-02-22 07:35:02]

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

[commit-bot: I haz the power, 2017-02-22 07:35:03]

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

[yhirano, 2017-02-22 07:40:51]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2017-02-22 07:41:40]

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-02-22 09:43:52]

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

[commit-bot: I haz the power, 2017-02-22 09:43:54]

Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[yhirano, 2017-02-22 14:00:17]

The CQ bit was checked by yhirano@chromium.org

[commit-bot: I haz the power, 2017-02-22 14:00:39]

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-02-22 14:51:22]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487772017323520,
"parent_rev": "0932b390e4b08af8590e8553a7b2a1fc8ef9c99b", "commit_rev":
"d4c7568248450ac03286a4af6fe46380b582ed68"}

[commit-bot: I haz the power, 2017-02-22 14:51:58]

Description was changed from

==========
ParsedContentType refactoring

This is a preliminary refactoring for fixing compatibility issues of
ParsedContentType.

BUG=689731
==========

to

==========
ParsedContentType refactoring

This is a preliminary refactoring for fixing compatibility issues of
ParsedContentType.

BUG=689731

Review-Url: https://codereview.chromium.org/2704153002
Cr-Commit-Position: refs/heads/master@{#452045}
Committed:
https://chromium.googlesource.com/chromium/src/+/d4c7568248450ac03286a4af6fe4...
==========

[commit-bot: I haz the power, 2017-02-22 14:52:00]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/d4c7568248450ac03286a4af6fe4...