Move NaCl loader related files to components/nacl/loader.

Move NaCl loader related files to components/nacl/loader.

nacl_exe_win_64.cc and nacl_helper_linux.cc will not move until breakpad is split off of chrome. 
nacl.gypi will move in a separate CL.

This is part of an effort to componentize NaCl code. 

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

[yael.aharon1, 2013-07-25 19:58:18]

This CL simply moves the files that are in the NaCl loader process to
components/nacl/loader. The new location was proposed in
https://codereview.chromium.org/16881004/#msg6 .
The move required some changes to include statements and to the guards of the
header files.

As discussed before, nacl_exe_win_64.cc and nacl_helper_linux.cc will not move
until breakpad is split off of chrome. Also, I will move nacl.gypi separately.

Mark, can you please review? If this still seems too much for one CL, please let
me know.

[Mark Seaborn, 2013-07-25 21:46:10]

> As discussed before, nacl_exe_win_64.cc and nacl_helper_linux.cc will
> not move until breakpad is split off of chrome. Also, I will move
> nacl.gypi separately.

Please put that information into the commit message.

https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit....
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit....
chrome/chrome_tests_unit.gypi:1838:
'../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
You've put these under the comment:

"# TODO(caitkp): Move to //components/components_tests.gypi once
 # remaining dependencies back to //chrome are eliminated."

But nacl_ipc_adapter_unittest.cc and nacl_validation_query_unittest.cc don't
have a dependency on chrome/.  So surely these unit tests can be moved to
components_tests.gypi already?

[yael.aharon1, 2013-07-25 22:29:13]

On 2013/07/25 21:46:10, Mark Seaborn wrote:
> > As discussed before, nacl_exe_win_64.cc and nacl_helper_linux.cc will
> > not move until breakpad is split off of chrome. Also, I will move
> > nacl.gypi separately.
> 
> Please put that information into the commit message.
Done

> 
>
https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit....
> File chrome/chrome_tests_unit.gypi (right):
> 
>
https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit....
> chrome/chrome_tests_unit.gypi:1838:
> '../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
> You've put these under the comment:
> 
> "# TODO(caitkp): Move to //components/components_tests.gypi once
>  # remaining dependencies back to //chrome are eliminated."
> 
> But nacl_ipc_adapter_unittest.cc and nacl_validation_query_unittest.cc don't
> have a dependency on chrome/.  So surely these unit tests can be moved to
> components_tests.gypi already?

These unit tests don't have a dependency on chrome, but they do have a
dependency on nacl.gypi, right? So I can't move the unit tests to
components_tests.gypi before moving nacl.gypi.

[yael.aharon1, 2013-07-26 12:17:55]

On 2013/07/25 21:46:10, Mark Seaborn wrote:
>
https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit....
> File chrome/chrome_tests_unit.gypi (right):
> 
>
https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit....
> chrome/chrome_tests_unit.gypi:1838:
> '../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
> You've put these under the comment:
> 
> "# TODO(caitkp): Move to //components/components_tests.gypi once
>  # remaining dependencies back to //chrome are eliminated."
> 
> But nacl_ipc_adapter_unittest.cc and nacl_validation_query_unittest.cc don't
> have a dependency on chrome/.  So surely these unit tests can be moved to
> components_tests.gypi already?

Actually, compiling the unit tests still depends on chrome layer defining the
variable "nacl_defines" which is currently defined in chrome.gyp. 
I will need to move that into another gyp file as proposed by Joi in
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/edUBehrcnEs...
.

[Mark Seaborn, 2013-07-26 16:49:52]

On 26 July 2013 05:17, <yael.aharon@chromium.org> wrote:

> On 2013/07/25 21:46:10, Mark Seaborn wrote:
>
> https://codereview.chromium.**org/20337002/diff/26001/**
>
chrome/chrome_tests_unit.gypi<https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit.gypi>
>
>> File chrome/chrome_tests_unit.gypi (right):
>>
>
>
> https://codereview.chromium.**org/20337002/diff/26001/**
>
chrome/chrome_tests_unit.gypi#**newcode1838<https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit.gypi#newcode1838>
>
>> chrome/chrome_tests_unit.gypi:**1838:
>> '../components/nacl/loader/**nacl_ipc_adapter_unittest.cc',
>> You've put these under the comment:
>>
>
>  "# TODO(caitkp): Move to //components/components_tests.**gypi once
>>   # remaining dependencies back to //chrome are eliminated."
>>
>
>  But nacl_ipc_adapter_unittest.cc and nacl_validation_query_**unittest.cc
>> don't
>> have a dependency on chrome/.  So surely these unit tests can be moved to
>> components_tests.gypi already?
>>
>
> Actually, compiling the unit tests still depends on chrome layer defining
> the
> variable "nacl_defines" which is currently defined in chrome.gyp.
> I will need to move that into another gyp file as proposed by Joi in
> https://groups.google.com/a/**chromium.org/forum/#!msg/**
>
chromium-dev/edUBehrcnEs/oMY_**d1mUngsJ<https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/edUBehrcnEs/oMY_d1mUngsJ>
> .
>

OK, that makes sense.  How about adding a TODO comment for these two lines
explaining the Gyp dependency on chrome/?  Leaving this under a TODO with
someone else's name on it doesn't seem fair to that person. :-)  Also, the
files listed under the TODO contain an obvious #include dependency on
chrome/, but the NaCl files' Gyp-level dependency isn't obvious.

Mark

[Mark Seaborn, 2013-07-26 16:49:52]

On 26 July 2013 05:17, <yael.aharon@chromium.org> wrote:

> On 2013/07/25 21:46:10, Mark Seaborn wrote:
>
> https://codereview.chromium.**org/20337002/diff/26001/**
>
chrome/chrome_tests_unit.gypi<https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit.gypi>
>
>> File chrome/chrome_tests_unit.gypi (right):
>>
>
>
> https://codereview.chromium.**org/20337002/diff/26001/**
>
chrome/chrome_tests_unit.gypi#**newcode1838<https://codereview.chromium.org/20337002/diff/26001/chrome/chrome_tests_unit.gypi#newcode1838>
>
>> chrome/chrome_tests_unit.gypi:**1838:
>> '../components/nacl/loader/**nacl_ipc_adapter_unittest.cc',
>> You've put these under the comment:
>>
>
>  "# TODO(caitkp): Move to //components/components_tests.**gypi once
>>   # remaining dependencies back to //chrome are eliminated."
>>
>
>  But nacl_ipc_adapter_unittest.cc and nacl_validation_query_**unittest.cc
>> don't
>> have a dependency on chrome/.  So surely these unit tests can be moved to
>> components_tests.gypi already?
>>
>
> Actually, compiling the unit tests still depends on chrome layer defining
> the
> variable "nacl_defines" which is currently defined in chrome.gyp.
> I will need to move that into another gyp file as proposed by Joi in
> https://groups.google.com/a/**chromium.org/forum/#!msg/**
>
chromium-dev/edUBehrcnEs/oMY_**d1mUngsJ<https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/edUBehrcnEs/oMY_d1mUngsJ>
> .
>

OK, that makes sense.  How about adding a TODO comment for these two lines
explaining the Gyp dependency on chrome/?  Leaving this under a TODO with
someone else's name on it doesn't seem fair to that person. :-)  Also, the
files listed under the TODO contain an obvious #include dependency on
chrome/, but the NaCl files' Gyp-level dependency isn't obvious.

Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to native-client-reviews+unsubscribe@googlegroups.com.
To post to this group, send email to native-client-reviews@googlegroups.com.
Visit this group at http://groups.google.com/group/native-client-reviews.
For more options, visit https://groups.google.com/groups/opt_out.

[Mark Seaborn, 2013-07-26 17:04:40]

Fix "separatel" typo in commit message.  Then LGTM with changes below.

https://codereview.chromium.org/20337002/diff/39001/chrome/chrome_tests_unit....
File chrome/chrome_tests_unit.gypi (right):

https://codereview.chromium.org/20337002/diff/39001/chrome/chrome_tests_unit....
chrome/chrome_tests_unit.gypi:1840:
'../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
Please add a TODO comment here as described in my previous message.

https://codereview.chromium.org/20337002/diff/39001/chrome/chrome_tests_unit....
chrome/chrome_tests_unit.gypi:1978:
'../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
And add a similar TODO comment here too so that this doesn't get missed.

https://codereview.chromium.org/20337002/diff/39001/components/nacl/OWNERS
File components/nacl/OWNERS (right):

https://codereview.chromium.org/20337002/diff/39001/components/nacl/OWNERS#ne...
components/nacl/OWNERS:8: per-file nacl_sandbox_linux.*=jln@chromium.org
Please remove the old nacl_sandbox_linux.cc entry from chrome/nacl/OWNERS.

[yael.aharon1, 2013-07-26 17:20:39]

On 2013/07/26 17:04:40, Mark Seaborn wrote:
> Fix "separatel" typo in commit message.  Then LGTM with changes below.
> 
>
https://codereview.chromium.org/20337002/diff/39001/chrome/chrome_tests_unit....
> File chrome/chrome_tests_unit.gypi (right):
> 
>
https://codereview.chromium.org/20337002/diff/39001/chrome/chrome_tests_unit....
> chrome/chrome_tests_unit.gypi:1840:
> '../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
> Please add a TODO comment here as described in my previous message.
> 
>
https://codereview.chromium.org/20337002/diff/39001/chrome/chrome_tests_unit....
> chrome/chrome_tests_unit.gypi:1978:
> '../components/nacl/loader/nacl_ipc_adapter_unittest.cc',
> And add a similar TODO comment here too so that this doesn't get missed.
> 
> https://codereview.chromium.org/20337002/diff/39001/components/nacl/OWNERS
> File components/nacl/OWNERS (right):
> 
>
https://codereview.chromium.org/20337002/diff/39001/components/nacl/OWNERS#ne...
> components/nacl/OWNERS:8: per-file
mailto:nacl_sandbox_linux.*=jln@chromium.org
> Please remove the old nacl_sandbox_linux.cc entry from chrome/nacl/OWNERS.

Added TODO, I obviously did not mean for someone else to fix this. I am
struggling with the gyp file in https://codereview.chromium.org/20550003/ , and
any advice would be appreciated :)

[yael.aharon1, 2013-07-26 17:21:01]

David, can you please review the change in the DPES file for ppapi ?
Basically, the code has moved from chrome/nacl to components/nacl/loader, so I
needed to move the entries in the DPES file.
thanks

[yael.aharon1, 2013-07-26 17:22:26]

jam, can you please review the change in the DEPS file for content/public/app ?

[yael.aharon1, 2013-07-26 17:24:25]

Justin, can you please review the change to the DEPS file for the sandbox
folder?
I moved the files from chrome/nacl to components/nacl/loader and needed to move
the entry in the DPES file.
thanks!

[yael.aharon1, 2013-07-26 17:27:35]

wtc@, can you please review the change in the DEPS file for crypto folder?
I moved files from chrome/nacl to components/nacl/loader and needed to add an
entry for that.
thanks!

[dmichael (off chromium), 2013-07-26 20:21:22]

ppapi-related DEPS changes lgtm

[wtc, 2013-07-26 21:54:32]

Patch set 3 LGTM. I only reviewed components/nacl/loader/DEPS.

[jam, 2013-07-26 22:35:21]

components/nacl/loader/DEPS lgtm

[yael.aharon1, 2013-07-27 11:47:56]

Justin, can you please review the addition of "+sandbox" directories to
components/nacl/loader? I moved some files from chrome/nacl to that location, so
I needed to copy over the dependency. Once all files are moved, I will delete
the dependency from the old location.
thanks

[yael.aharon1, 2013-07-30 11:31:59]

cpu@ can you please approve the sandbox entries in the DPES file?
I moved files from chrome/nacl to components/nacl/loader so I needed to copy the
sandbox related entries from chrome/nacl/DEPS to components/nacl/loader/DEPS.
Once I finish moving all the files, the old entries will be deleted.

Everything else in this CL has been approved already.
thanks!

[yael.aharon1, 2013-07-30 11:35:46]

cpu@ can you please approve the sandbox entries in the DPES file?
I moved files from chrome/nacl to components/nacl/loader so I needed to copy the
sandbox related entries from chrome/nacl/DEPS to components/nacl/loader/DEPS.
Once I finish moving all the files, the old entries will be deleted.

Everything else in this CL has been approved already.
thanks!

[cpu_(ooo_6.6-7.5), 2013-07-30 18:25:27]

lgtm

[commit-bot: I haz the power, 2013-07-30 20:45:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yael.aharon@intel.com/20337002/45002

[commit-bot: I haz the power, 2013-07-31 05:07:04]

Change committed as 214567