NaCl cleanup: Split out irt_interfaces.cc from irt_ppapi.cc

NaCl cleanup: Split out irt_interfaces.cc from irt_ppapi.cc

The intention here is that irt_interfaces.{cc,h} should list the NaCl
IRT interfaces that we implement in Chromium but that other files
(like irt_ppapi.cc) should implement them.  This matches how the files
are organised in native_client/src/untrusted/irt/.

This is in preparation for copying the PNaCl translator IRT interfaces
to the Chromium side.  That will add a couple more interfaces to
irt_interfaces.cc (rather than to irt_ppapi.cc).

BUG=302078
TEST=NaCl tests in browser_tests

Committed: https://crrev.com/5e662010c1f7881970de6de19bcbd50f2726ea00
Cr-Commit-Position: refs/heads/master@{#339508}

[Mark Seaborn, 2015-07-17 22:50:40]

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

[commit-bot: I haz the power, 2015-07-17 22:51:25]

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

[commit-bot: I haz the power, 2015-07-18 00:02:57]

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

[commit-bot: I haz the power, 2015-07-18 00:02:58]

Dry run: This issue passed the CQ dry run.

[Mark Seaborn, 2015-07-20 15:08:33]

mseaborn@chromium.org changed reviewers:
+ jvoung@chromium.org

[jvoung (off chromium), 2015-07-20 16:01:04]

lgtm

[Mark Seaborn, 2015-07-20 16:02:01]

The CQ bit was checked by mseaborn@chromium.org

[commit-bot: I haz the power, 2015-07-20 16:02:44]

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

[commit-bot: I haz the power, 2015-07-20 16:09:32]

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

[commit-bot: I haz the power, 2015-07-20 16:09:32]

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

[Mark Seaborn, 2015-07-20 16:41:20]

mseaborn@chromium.org changed reviewers:
+ bbudge@chromium.org

[Mark Seaborn, 2015-07-20 16:41:55]

+bbudge for OWNERS review of ppapi/

[bbudge, 2015-07-20 17:21:09]

https://codereview.chromium.org/1244533006/diff/1/ppapi/nacl_irt/irt_interfac...
File ppapi/nacl_irt/irt_interfaces.h (right):

https://codereview.chromium.org/1244533006/diff/1/ppapi/nacl_irt/irt_interfac...
ppapi/nacl_irt/irt_interfaces.h:10: extern const struct nacl_irt_ppapihook
nacl_irt_ppapihook;
Is there a reason this is declared here and not in irt_ppapi.h? It's defined in
irt_ppapi.cc.

[Mark Seaborn, 2015-07-20 17:49:08]

https://codereview.chromium.org/1244533006/diff/1/ppapi/nacl_irt/irt_interfac...
File ppapi/nacl_irt/irt_interfaces.h (right):

https://codereview.chromium.org/1244533006/diff/1/ppapi/nacl_irt/irt_interfac...
ppapi/nacl_irt/irt_interfaces.h:10: extern const struct nacl_irt_ppapihook
nacl_irt_ppapihook;
On 2015/07/20 17:21:09, bbudge wrote:
> Is there a reason this is declared here and not in irt_ppapi.h? It's defined
in
> irt_ppapi.cc.

The idea was to copy how it's done in
native_client/src/untrusted/irt/irt_interfaces.h.

I'll be adding irt_pnacl_translator_{compile,link}.cc in a follow-on change. 
While I could put the struct decls in separate .h files, that leads to having a
lot of .h files containing one decl each.

[bbudge, 2015-07-20 20:05:07]

lgtm

[Mark Seaborn, 2015-07-20 20:05:25]

The CQ bit was checked by mseaborn@chromium.org

[commit-bot: I haz the power, 2015-07-20 20:06:52]

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

[commit-bot: I haz the power, 2015-07-20 20:42:00]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-07-20 20:44:00]

Patchset 1 (id:??) landed as
https://crrev.com/5e662010c1f7881970de6de19bcbd50f2726ea00
Cr-Commit-Position: refs/heads/master@{#339508}

[Ryan Sleevi, 2015-07-20 23:19:21]

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

[Ryan Sleevi, 2015-07-20 23:19:21]

It looks like this regressed Linux Sizes -
https://build.chromium.org/p/chromium/builders/Linux/builds/64689

[Ryan Sleevi, 2015-07-20 23:24:30]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/1249473003/ by rsleevi@chromium.org.

The reason for reverting is: Regressed Linux sizes - 4.5% is a bit of a hit to
take to bump sizes. If you're absolutely sure this is acceptable, update sizes
as well..

[Mark Seaborn, 2015-07-21 00:25:11]

On 20 July 2015 at 16:19, <rsleevi@chromium.org> wrote:

> It looks like this regressed Linux Sizes -
> https://build.chromium.org/p/chromium/builders/Linux/builds/64689


The revert does not seem to have fixed the problem.

This build has the same size error:
https://build.chromium.org/p/chromium/builders/Linux/builds/64692
and it includes the revert (https://codereview.chromium.org/1249473003).

Mark

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

[Mark Seaborn, 2015-07-21 04:16:40]

On 20 July 2015 at 17:24, Mark Seaborn <mseaborn@chromium.org> wrote:

> On 20 July 2015 at 16:19, <rsleevi@chromium.org> wrote:
>
>> It looks like this regressed Linux Sizes -
>> https://build.chromium.org/p/chromium/builders/Linux/builds/64689
>
>
> The revert does not seem to have fixed the problem.
>
> This build has the same size error:
> https://build.chromium.org/p/chromium/builders/Linux/builds/64692
> and it includes the revert (https://codereview.chromium.org/1249473003).
>

I investigated.  The size increase appears to be caused by this Skia DEPS
roll:
https://codereview.chromium.org/1245793002

Before that change (at 3ab481ec3953f63cfc1ec066eafd788d061f7834), I get:
$ size out/Release/nacl_helper
   text    data     bss     dec     hex filename
6564558  234696  267544 7066798  6bd4ae out/Release/nacl_helper

After that change (at 56b2dde5e64a5faea6561a900cf3f27bde3d7ad9), I get:
$ size out/Release/nacl_helper
   text    data     bss     dec     hex filename
6704695  234696  267544 7206935  6df817 out/Release/nacl_helper

Cheers,
Mark

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