Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(219)

Issue 147803003: NaCl docs: add ARM 32-bit sandbox (Closed)

Created:
6 years, 10 months ago by JF
Modified:
6 years, 8 months ago
CC:
chromium-reviews, native-client-reviews_googlegroups.com, binji, Sam Clegg
Visibility:
Public.

Description

NaCl docs: add ARM 32-bit sandbox As discussed we'll add sandbox documentation to our reference section. This CL adds details on the ARM 32-bit sandbox, and is mostly a port of the following wiki page: http://www.chromium.org/nativeclient/reference/arm-overview It also updates some parts of this page. I'll delete the wiki page once the document is live. R= binji@chromium.org, sbc@chromium.org, kschimpf@chromium.org, awatson@chromium.org, mackinlay@chromium.org BUG= none TEST= none NOTRY=true (documentation only change) Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=249773

Patch Set 1 #

Total comments: 24

Patch Set 2 : Update code blocks, and add generated files. #

Patch Set 3 : Address binji's comments. #

Total comments: 12
Unified diffs Side-by-side diffs Delta from patch set Stats (+1750 lines, -0 lines) Patch
A native_client_sdk/doc_generated/reference/sandbox_internals/arm-32-bit-sandbox.html View 1 2 1 chunk +812 lines, -0 lines 0 comments Download
A native_client_sdk/doc_generated/reference/sandbox_internals/index.html View 1 1 chunk +12 lines, -0 lines 0 comments Download
M native_client_sdk/doc_generated/sitemap.html View 1 1 chunk +6 lines, -0 lines 0 comments Download
M native_client_sdk/src/doc/_book.yaml View 1 chunk +4 lines, -0 lines 2 comments Download
A native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst View 1 2 1 chunk +903 lines, -0 lines 6 comments Download
A native_client_sdk/src/doc/reference/sandbox_internals/index.rst View 1 chunk +11 lines, -0 lines 4 comments Download
M native_client_sdk/src/doc/sitemap.rst View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
JF
6 years, 10 months ago (2014-02-06 17:18:20 UTC) #1
JF
I updated the code blocks: - Some were too wide and wrapped around. - I ...
6 years, 10 months ago (2014-02-06 19:10:07 UTC) #2
binji
lgtm I'd like to see the diffs on the generated documents, if you can. Maybe ...
6 years, 10 months ago (2014-02-06 21:23:39 UTC) #3
Karl
lgtm
6 years, 10 months ago (2014-02-06 21:56:26 UTC) #4
JF
https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst File native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst (right): https://codereview.chromium.org/147803003/diff/1/native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst#newcode241 native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:241: Alternative Sandboxing On 2014/02/06 21:23:39, binji wrote: > It's ...
6 years, 10 months ago (2014-02-06 23:26:35 UTC) #5
JF
The CQ bit was checked by jfb@chromium.org
6 years, 10 months ago (2014-02-07 19:27:41 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jfb@chromium.org/147803003/130001
6 years, 10 months ago (2014-02-07 19:30:59 UTC) #7
Andy
LGTM with a few small comments inline. https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/doc/_book.yaml File native_client_sdk/src/doc/_book.yaml (right): https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/doc/_book.yaml#newcode1 native_client_sdk/src/doc/_book.yaml:1: toc: This ...
6 years, 10 months ago (2014-02-07 19:41:13 UTC) #8
commit-bot: I haz the power
Change committed as 249773
6 years, 10 months ago (2014-02-07 20:52:03 UTC) #9
JF
6 years, 10 months ago (2014-02-07 21:15:14 UTC) #10
Message was sent while issue was closed.
Sorry Andy, I missed your comments before checking the commit box. I addressed
them in this CL instead:
  https://codereview.chromium.org/144253005

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File native_client_sdk/src/doc/_book.yaml (right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/_book.yaml:1: toc:
On 2014/02/07 19:41:13, Andy wrote:
> This file is being deprecated with the move to Chromesite.  Please update the
> following file instead:
> 
> /src/chrome/common/extensions/docs/templates/json/chrome_sidenav.json

That's pretty unfortunate: as a non-frequent contributor to documentation my
approach is to look at what other pages in our docs do, and then grep around in
native_client_sdk/docs. I didn't find this other sidenav file because it's
somewhere else!

Is there a way to fix this so that all the NaCl documentation is under
native_client_sdk/docs.

I'll update that file for now, it's just bound to get out of date IMO.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst
(right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:5:
Native Client for ARM is a method for running programs---even malicious
On 2014/02/07 19:41:13, Andy wrote:
> "method" --> "sandboxing technology"

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:6:
ones---safely, on computers that use 32-bit ARM processors. It's an
On 2014/02/07 19:41:13, Andy wrote:
> "It's" --> "The ARM sandbox is"

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/arm-32-bit-sandbox.rst:7:
extension of earlier work on Native Client for x86 processors. This
On 2014/02/07 19:41:13, Andy wrote:
> "This security" --> "Security"

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
File native_client_sdk/src/doc/reference/sandbox_internals/index.rst (right):

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/index.rst:7: The sandbox
internals explains implementation details about Native
On 2014/02/07 19:41:13, Andy wrote:
> Suggest beginning with:
> """
> The sandbox internals documentation describes implementation details for
Native
> Client sandboxing,
> """

Done.

https://codereview.chromium.org/147803003/diff/130001/native_client_sdk/src/d...
native_client_sdk/src/doc/reference/sandbox_internals/index.rst:11: Client
doesn't allow developers to write platform-specific assembly).
On 2014/02/07 19:41:13, Andy wrote:
> "doesn't allow developers to write platform-specific assembly" --> "does not
> allow platform-specific assembly code"

Done.

Powered by Google App Engine
This is Rietveld 408576698