Pepper: Fix caching for translated nexes.

Pepper: Fix caching for translated nexes.

A refactor of FileDownloader (r284961) broke the caching behavior for nexes
that were translated from pexes; the cache would always end up with entries of
length 0, breaking future loads of the pexe. This is the translated nexe was
written to the wrong file handle, preventing caching of the result.

This change causes the translated nexe to be written to the correct file
handle, fixing the caching behavior.

I manually tested this change on the "game of life" demo at
https://gonativeclient.appspot.com/demo/life, verifying that a cached
translated nexe for the pexe was successfully loaded.

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

[dmichael (off chromium), 2014-08-06 20:13:24]

lgtm

I think users affected by this will still get bad nexes from their cache until
they expire, right? Maybe it isn't worth writing code to address that case, but
it would at least be a good idea to give instructions on the
native-client-discuss thread to work around it. WDYT?

[teravest, 2014-08-06 20:15:55]

On Wed, Aug 6, 2014 at 2:13 PM,  <dmichael@chromium.org> wrote:
> lgtm
>
> I think users affected by this will still get bad nexes from their cache
> until
> they expire, right? Maybe it isn't worth writing code to address that case,
> but
> it would at least be a good idea to give instructions on the
> native-client-discuss thread to work around it. WDYT?

That's correct. I can't imagine any case where a good translated nexe
would have size 0, so I could treat any cache hit as a miss if the
file length is zero. That doesn't seem like too ugly of a hack to keep
around.

>
> https://codereview.chromium.org/447013002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[teravest, 2014-08-06 20:31:09]

On 2014/08/06 20:15:55, teravest wrote:
> On Wed, Aug 6, 2014 at 2:13 PM,  <mailto:dmichael@chromium.org> wrote:
> > lgtm
> >
> > I think users affected by this will still get bad nexes from their cache
> > until
> > they expire, right? Maybe it isn't worth writing code to address that case,
> > but
> > it would at least be a good idea to give instructions on the
> > native-client-discuss thread to work around it. WDYT?
> 
> That's correct. I can't imagine any case where a good translated nexe
> would have size 0, so I could treat any cache hit as a miss if the
> file length is zero. That doesn't seem like too ugly of a hack to keep
> around.

After discussing this, the file creation path is different for cache hits and
cache misses. I'm not totally sure it's okay to have the plugin write to a file
that was designated as something retrieved out of the cache, and I don't want to
make this code more complicated for a relatively short window.

Once this goes in, I'll send an announcement telling users how to clear their
cache (and the affected versions of chrome).

> 
> >
> > https://codereview.chromium.org/447013002/
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[teravest, 2014-08-06 20:31:43]

The CQ bit was checked by teravest@chromium.org

[commit-bot: I haz the power, 2014-08-06 20:32:59]

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

[Derek Schuff, 2014-08-06 20:55:49]

On 2014/08/06 20:32:59, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-status.appspot.com/cq/teravest@chromium.org/447013002/1

LGTM too. It would be non-trivial to just treat a hit with 0 length as a miss
because by the time you know you'd have the cache entry open in the browser
side, and you'd have to close/purge it and also invoke the miss-handling logic.
You probably could keep it all on the browser side which would be simpler, but
it's probably still not worth it. things sometimes break on canary, and as our
friend Victor says, "you get to keep both pieces". We should just reply to the
native-client-discuss thread that people can just clear the cache via the
browser UI.

also, please do CC me or Jan for changes to pnacl coordinator, translation
streaming, etc.

[commit-bot: I haz the power, 2014-08-07 00:15:46]

Change committed as 287912