Check for errors when calling ZipReader::AdvanceToNextEntry.

Check for errors when calling ZipReader::AdvanceToNextEntry in Safe Browsing download checks.

BUG=228869
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=193726

[mattm, 2013-04-10 20:05:22]

https://codereview.chromium.org/14080002/diff/1/chrome/common/safe_browsing/z...
File chrome/common/safe_browsing/zip_analyzer.cc (right):

https://codereview.chromium.org/14080002/diff/1/chrome/common/safe_browsing/z...
chrome/common/safe_browsing/zip_analyzer.cc:23: if (!advanced) {
maybe cleaner to use a while loop like:
while (hasmore()) {
  if (!advance()) {
    ...
    return;
  }

[mattm, 2013-04-10 20:05:46]

Also mention safebrowsing in the commit description

[Brian Ryner, 2013-04-10 20:11:46]

Updated the change description.

Thanks,

https://codereview.chromium.org/14080002/diff/1/chrome/common/safe_browsing/z...
File chrome/common/safe_browsing/zip_analyzer.cc (right):

https://codereview.chromium.org/14080002/diff/1/chrome/common/safe_browsing/z...
chrome/common/safe_browsing/zip_analyzer.cc:23: if (!advanced) {
On 2013/04/10 20:05:22, mattm wrote:
> maybe cleaner to use a while loop like:
> while (hasmore()) {
>   if (!advance()) {
>     ...
>     return;
>   }

I'm not sure how much cleaner that turns out to be, since the reader is
initially positioned at the first entry (i.e. we don't want to advance on the
first time through the loop).

[mattm, 2013-04-10 20:23:27]

lgtm

https://codereview.chromium.org/14080002/diff/1/chrome/common/safe_browsing/z...
File chrome/common/safe_browsing/zip_analyzer.cc (right):

https://codereview.chromium.org/14080002/diff/1/chrome/common/safe_browsing/z...
chrome/common/safe_browsing/zip_analyzer.cc:23: if (!advanced) {
On 2013/04/10 20:11:47, Brian Ryner wrote:
> On 2013/04/10 20:05:22, mattm wrote:
> > maybe cleaner to use a while loop like:
> > while (hasmore()) {
> >   if (!advance()) {
> >     ...
> >     return;
> >   }
> 
> I'm not sure how much cleaner that turns out to be, since the reader is
> initially positioned at the first entry (i.e. we don't want to advance on the
> first time through the loop).
> 

ah, right.

[commit-bot: I haz the power, 2013-04-10 20:27:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryner@chromium.org/14080002/1

[commit-bot: I haz the power, 2013-04-10 22:01:17]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-04-10 22:02:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryner@chromium.org/14080002/1

[commit-bot: I haz the power, 2013-04-11 06:25:56]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-04-11 19:58:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bryner@chromium.org/14080002/1

[commit-bot: I haz the power, 2013-04-11 20:23:48]

Change committed as 193726