Trim whitespace from parsed filename in Android v21.

Trim whitespace from parsed filename in Android v21.

The entire text content of the 'font' element is currently used as the
file name. This wasn't an issue in earlier versions, as the 'file'
element was dedicated to only containing the file name. The new 'font'
element also contains a number of attributes and potentially other tags.
This means that a 'font' element can become quite long, making it
desireable to be able to split it across multiple lines.

However, splitting the 'font' element across multiple lines is currently
difficult and awkward as any whitespace outside of tags will be
considered part of the file name. This change means that any leading or
trailing whitespace will not be considered part of the file name.

This only applies to v21 and later files, so while this restricts font
file names from beginning and ending with whitespace, it is unlikely to
break any users in practice. It is probably also undesireable to have
font files with names that begin or end with whitespace in any event.

Committed: https://skia.googlesource.com/skia/+/c0727d117e67844f0c8794cc7eaa49a96a015347

[bungeman-skia, 2015-05-07 20:49:43]

bungeman@google.com changed reviewers:
+ djsollen@google.com

[bungeman-skia, 2015-05-07 20:49:50]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-07 20:50:24]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1125413003/1

[commit-bot: I haz the power, 2015-05-07 20:56:54]

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

[commit-bot: I haz the power, 2015-05-07 20:56:55]

Dry run: This issue passed the CQ dry run.

[bungeman-skia, 2015-05-07 21:24:31]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-07 21:24:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1125413003/20001

[commit-bot: I haz the power, 2015-05-07 21:35:40]

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

[commit-bot: I haz the power, 2015-05-07 21:35:41]

Dry run: This issue passed the CQ dry run.

[djsollen, 2015-05-08 13:56:13]

https://codereview.chromium.org/1125413003/diff/20001/src/ports/SkFontConfigP...
File src/ports/SkFontConfigParser_android.cpp (right):

https://codereview.chromium.org/1125413003/diff/20001/src/ports/SkFontConfigP...
src/ports/SkFontConfigParser_android.cpp:270: for (; *start == ' ' || *start ==
'\n'|| *start == '\r' || *start == '\t'; ++start) { }
while I can appreciate the compactness of this it is non-trivial to read.

static bool whitespace_char(const char& c) {
  return *start == ' ' || *start == '\n'|| *start == '\r' || *start == '\t';
}

while (!whitespace_char(*start)) { ++start; }

https://codereview.chromium.org/1125413003/diff/20001/src/ports/SkFontConfigP...
src/ports/SkFontConfigParser_android.cpp:272: for (--end; *end == ' ' || *end ==
'\n'|| *end == '\r' || *end == '\t'; --end) { }
comment why you need the --end at the begin and end of this if statement.

[bungeman-skia, 2015-05-08 15:05:04]

Perhaps this is somewhat less inscrutable?

https://codereview.chromium.org/1125413003/diff/20001/src/ports/SkFontConfigP...
File src/ports/SkFontConfigParser_android.cpp (right):

https://codereview.chromium.org/1125413003/diff/20001/src/ports/SkFontConfigP...
src/ports/SkFontConfigParser_android.cpp:270: for (; *start == ' ' || *start ==
'\n'|| *start == '\r' || *start == '\t'; ++start) { }
On 2015/05/08 13:56:13, djsollen wrote:
> while I can appreciate the compactness of this it is non-trivial to read.
> 
> static bool whitespace_char(const char& c) {
>   return *start == ' ' || *start == '\n'|| *start == '\r' || *start == '\t';
> }
> 
> while (!whitespace_char(*start)) { ++start; }

Done.

https://codereview.chromium.org/1125413003/diff/20001/src/ports/SkFontConfigP...
src/ports/SkFontConfigParser_android.cpp:272: for (--end; *end == ' ' || *end ==
'\n'|| *end == '\r' || *end == '\t'; --end) { }
On 2015/05/08 13:56:13, djsollen wrote:
> comment why you need the --end at the begin and end of this if statement.

Done.

[bungeman-skia, 2015-05-08 15:10:34]

The CQ bit was checked by bungeman@google.com to run a CQ dry run

[commit-bot: I haz the power, 2015-05-08 15:10:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1125413003/40001

[commit-bot: I haz the power, 2015-05-08 15:16:42]

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

[commit-bot: I haz the power, 2015-05-08 15:16:43]

Dry run: This issue passed the CQ dry run.

[djsollen, 2015-05-08 15:30:02]

lgtm

[bungeman-skia, 2015-05-08 15:30:51]

The CQ bit was checked by bungeman@google.com

[commit-bot: I haz the power, 2015-05-08 15:31:04]

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

[commit-bot: I haz the power, 2015-05-08 15:32:01]

Committed patchset #3 (id:40001) as
https://skia.googlesource.com/skia/+/c0727d117e67844f0c8794cc7eaa49a96a015347