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

Issue 1046323002: Revert "Added support for NaCl IO to use the IRT Extension API." (Closed)

Created:
5 years, 8 months ago by binji
Modified:
5 years, 8 months ago
Reviewers:
Sam Clegg
CC:
chromium-reviews, binji+watch_chromium.org, Sam Clegg
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert "Added support for NaCl IO to use the IRT Extension API." This reverts commit 3183230ac54a9b443f6e67a64121a0c3ee734e6b. There is some subtle behavior when certain interfaces are not available, and how libnacl falls-back to other interfaces. For example, the fdio interface is not available on PNaCl, but the fdio-dev interface is. libnacl has an early fallback that attempts to query fdio, and if it is not available, will fill in its fdio interface struct with the fdio-dev pointers. This logic is necessary to get output to the Chrome console. In kernel_wrap_newlib, this behavior is preserved because the "real" pointers are grabbed directly from the newlib structures. In The IRT extension API, this fails because it queries the interfaces directly. If we query the interface pointers directly, we'll have to reimplement the fall-back logic. Perhaps this is worth doing, but the IRT extension API is still not implemented for glibc, so the benefits of it are minimal at this point. BUG=http://crbug.com/472185 R=sbc@chromium.org CC=dyen@chromium.org Committed: https://crrev.com/8c7a8d9ac7b85e64f69df7956c461122057bad91 Cr-Commit-Position: refs/heads/master@{#323260}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+8 lines, -515 lines) Patch
M native_client_sdk/src/build_tools/build_sdk.py View 1 chunk +0 lines, -1 line 0 comments Download
M native_client_sdk/src/libraries/nacl_io/kernel_wrap.h View 1 chunk +0 lines, -6 lines 0 comments Download
M native_client_sdk/src/libraries/nacl_io/kernel_wrap_bionic.cc View 2 chunks +2 lines, -5 lines 0 comments Download
M native_client_sdk/src/libraries/nacl_io/kernel_wrap_glibc.cc View 1 chunk +3 lines, -3 lines 0 comments Download
D native_client_sdk/src/libraries/nacl_io/kernel_wrap_irt_ext.c View 1 chunk +0 lines, -496 lines 0 comments Download
M native_client_sdk/src/libraries/nacl_io/kernel_wrap_newlib.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M native_client_sdk/src/libraries/nacl_io/library.dsc View 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 11 (3 generated)
binji
5 years, 8 months ago (2015-04-01 00:16:09 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1046323002/1
5 years, 8 months ago (2015-04-01 00:17:09 UTC) #3
Sam Clegg
lgtm (reverted without any conflicts?)
5 years, 8 months ago (2015-04-01 00:20:59 UTC) #4
commit-bot: I haz the power
This issue passed the CQ dry run.
5 years, 8 months ago (2015-04-01 00:58:52 UTC) #6
binji
On 2015/04/01 00:20:59, Sam Clegg wrote: > lgtm (reverted without any conflicts?) yep!
5 years, 8 months ago (2015-04-01 16:52:18 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1046323002/1
5 years, 8 months ago (2015-04-01 16:52:42 UTC) #9
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 8 months ago (2015-04-01 16:54:01 UTC) #10
commit-bot: I haz the power
5 years, 8 months ago (2015-04-01 16:55:00 UTC) #11
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8c7a8d9ac7b85e64f69df7956c461122057bad91
Cr-Commit-Position: refs/heads/master@{#323260}

Powered by Google App Engine
This is Rietveld 408576698