Remove PNaCl's RequestFirstInstall, use resource throttle instead

Remove PNaCl's RequestFirstInstall, use resource throttle instead

This removes PNaCl's RequestFirstInstall and its use of
the component updater OnDemand API. A different CL:
https://codereview.chromium.org/25713007/
will do the OnDemand calls internally.

This removes the async IPCs, callbacks, and observers that
were used to support the old code. The new code just
throttles/blocks the pexe URL request until OnDemand
installation is done.

Reverts: https://codereview.chromium.org/19863003/, plus
other CLs.

BUG=none
R=cpu@chromium.org, dmichael@chromium.org
TBR=jln

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

[jvoung (off chromium), 2013-10-31 23:00:32]

I think this should remove PNaCl's call to the OnDemand API, along with the IPC
and callbacks that lead up to it.

I'll ask IPC owners and Pepper owners for reviewing those parts.

Also, when I tested this out with https://codereview.chromium.org/25713007/, it
seems like there is still two periods of sleep before downloading starts?

[cpu_(ooo_6.6-7.5), 2013-11-01 00:11:44]

If so that is bad there should be no sleeps. Let me do some debugging tomorrow.

[cpu_(ooo_6.6-7.5), 2013-11-01 17:28:59]

lgtm

https://codereview.chromium.org/55463002/diff/170001/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc (left):

https://codereview.chromium.org/55463002/diff/170001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc:449:
resources_->ReadResourceInfo(PnaclUrls::GetResourceInfoUrl(),
don't know where this moved to, just pointing it out since the resource info
file is not CU related.

[jvoung (off chromium), 2013-11-01 21:56:12]

https://codereview.chromium.org/55463002/diff/170001/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc (left):

https://codereview.chromium.org/55463002/diff/170001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc:449:
resources_->ReadResourceInfo(PnaclUrls::GetResourceInfoUrl(),
On 2013/11/01 17:28:59, cpu wrote:
> don't know where this moved to, just pointing it out since the resource info
> file is not CU related. 


This was moved to line 527 (where it originally started an async call that
eventually hits this function "DidCheckPnaclInstalled). Since
DidCheckPnaclInstalled is gone, it's there.

[jvoung (off chromium), 2013-11-01 23:05:44]

+dmichael for pepper, +jln for nacl_host_messages.h, thanks in advance

[Derek Schuff, 2013-11-01 23:36:28]

https://codereview.chromium.org/55463002/diff/450001/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc (left):

https://codereview.chromium.org/55463002/diff/450001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc:456: // Second step
of loading resources: call StartLoad.
the first step is gone, so fix this comment?

[dmichael (off chromium), 2013-11-02 08:40:45]

Pepper lgtm

[jvoung (off chromium), 2013-11-04 18:16:36]

https://codereview.chromium.org/55463002/diff/450001/ppapi/native_client/src/...
File ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc (left):

https://codereview.chromium.org/55463002/diff/450001/ppapi/native_client/src/...
ppapi/native_client/src/trusted/plugin/pnacl_coordinator.cc:456: // Second step
of loading resources: call StartLoad.
On 2013/11/01 23:36:28, Derek Schuff wrote:
> the first step is gone, so fix this comment?

In the new patch, this second step follows line 524, which still says "first
step".

I'll reorder the functions so that they follow each other in the right order.

[jvoung (off chromium), 2013-11-04 21:17:32]

jln to TBR, since this is just reverting/remove an earlier IPC that you reviewed

[waffles, 2013-11-04 22:07:26]

What is the behavior when we attempt an on-demand update of PNaCl, the update
fails (say clients2.google.com is unreachable), and the .pexe download is
unblocked and then completes?

[cpu_(ooo_6.6-7.5), 2013-11-04 22:19:56]

just early warning, we have 3 hours left.

[jvoung (off chromium), 2013-11-04 22:29:22]

On 2013/11/04 22:07:26, waffles wrote:
> What is the behavior when we attempt an on-demand update of PNaCl, the update
> fails (say http://clients2.google.com is unreachable), and the .pexe download
is
> unblocked and then completes?

The .pexe download unblocks, which allows the code in pnacl_coordinator.cc to be
called back.

It then proceeds to try and open and read "resource info" file, but that will
fail to open because it's not on disk.

Unless the website developer does something, the user will end up seeing nothing
except that the page doesn't load. However, we dispatch a JavaScript progress
event "error" along with a string "NaCl module load failed: The Portable Native
Client (pnacl) component is not installed..."  The developer could listen for
that JS progress event and display some actual in-page UI.

[jvoung (off chromium), 2013-11-04 22:35:01]

Committed patchset #5 manually as r232827 (presubmit successful).

[jvoung (off chromium), 2013-11-05 00:30:20]

On 2013/11/01 00:11:44, cpu wrote:
> If so that is bad there should be no sleeps. Let me do some debugging
tomorrow.

Hmm I didn't notice this earlier, because I mostly tried reloading from the same
browser session, but if you start a new browser session and PNaCl is already
installed, the first load now has a sleep before the pexe loads (vs what
happened before, where it just loads the pexe).

This is a combination of the sleeps and the throttle always doing a check. After
the first time, it doesn't need to sleep because the delta between the last
check < OnDemandDelay() and the OnDemand function just returns kError.

[waffles, 2013-11-05 00:39:15]

On 2013/11/05 00:30:20, jvoung (cr) wrote:
> On 2013/11/01 00:11:44, cpu wrote:
> > If so that is bad there should be no sleeps. Let me do some debugging
> tomorrow.
> 
> Hmm I didn't notice this earlier, because I mostly tried reloading from the
same
> browser session, but if you start a new browser session and PNaCl is already
> installed, the first load now has a sleep before the pexe loads (vs what
> happened before, where it just loads the pexe).
> 
> This is a combination of the sleeps and the throttle always doing a check.
After
> the first time, it doesn't need to sleep because the delta between the last
> check < OnDemandDelay() and the OnDemand function just returns kError.

It is at least just a few seconds' sleep, right? < 5s?
Note that we want to get rid of the OnDemandDelay soon.

[jvoung (off chromium), 2013-11-05 01:41:38]

On 2013/11/05 00:39:15, waffles wrote:
> On 2013/11/05 00:30:20, jvoung (cr) wrote:
> > On 2013/11/01 00:11:44, cpu wrote:
> > > If so that is bad there should be no sleeps. Let me do some debugging
> > tomorrow.
> > 
> > Hmm I didn't notice this earlier, because I mostly tried reloading from the
> same
> > browser session, but if you start a new browser session and PNaCl is already
> > installed, the first load now has a sleep before the pexe loads (vs what
> > happened before, where it just loads the pexe).
> > 
> > This is a combination of the sleeps and the throttle always doing a check.
> After
> > the first time, it doesn't need to sleep because the delta between the last
> > check < OnDemandDelay() and the OnDemand function just returns kError.
> 
> It is at least just a few seconds' sleep, right? < 5s?
> Note that we want to get rid of the OnDemandDelay soon.


Yeah, it's the 4s sleep from the CU main-loop StepDelay, plus the time it takes
for the server to say that there's no update. Still, 4s isn't exactly fast.

I'll see if I can have PNaCl opt-out of the per-pexe-load throttle + OnDemand
check when there's no need. Otherwise, once OnDemandDelay() is removed, every
PNaCl load will sleep and ping the server?

[jln (very slow on Chromium), 2013-11-05 03:55:51]

belated components/nacl/common/nacl_host_messages.h lgtm