Starboardize base #2182
Starboardize base #2182
Conversation
b/298237511
|
Note these changes are all already in feature/all-upstream-update, this is just a review for cleanliness. Nothing will be able to build yet as the update to //build is still in progress in #2100 |
| @@ -96,7 +97,7 @@ bool IsLargeMemoryDevice() { | |||
| } | |||
|
|
|||
| bool SetAddressSpaceLimit() { | |||
| #if !defined(ARCH_CPU_64_BITS) || !BUILDFLAG(IS_POSIX) | |||
| #if !defined(ARCH_CPU_64_BITS) || !BUILDFLAG(IS_POSIX) || 1 | |||
| @@ -124,7 +125,7 @@ bool SetAddressSpaceLimit() { | |||
| } | |||
|
|
|||
| bool ClearAddressSpaceLimit() { | |||
| #if !defined(ARCH_CPU_64_BITS) || !BUILDFLAG(IS_POSIX) | |||
| #if !defined(ARCH_CPU_64_BITS) || !BUILDFLAG(IS_POSIX) || 1 | |||
| @@ -621,7 +622,7 @@ void FreeFullSlotSpan(PartitionRoot<internal::ThreadSafe>* root, | |||
| EXPECT_TRUE(slot_span->is_empty()); | |||
| } | |||
|
|
|||
| #if BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) | |||
| #if 0// BUILDFLAG(IS_LINUX) || BUILDFLAG(IS_CHROMEOS) | |||
| @@ -341,6 +345,9 @@ absl::optional<std::vector<uint8_t>> ReadFileToBytes(const FilePath& path) { | |||
| })) { | |||
| return absl::nullopt; | |||
| } | |||
| #else | |||
| std::vector<uint8_t> bytes; | |||
There was a problem hiding this comment.
Add a NOTIMPLEMENTED() or equivalent help to help highlight when this no-op code is executed.
| ScopedFILE file_stream(OpenFile(path, "rb")); | ||
| if (!file_stream) | ||
| return false; | ||
| return ReadStreamToStringWithMaxSize(file_stream.get(), max_size, contents); | ||
| #endif | ||
| return false; |
There was a problem hiding this comment.
#else ... #endif so that this doesn't become unreachable code on non-cobalt builds.
And also add a NOTIMPLEMENTED() or equivalent help to help highlight when this no-op code is executed.
| #if !(BUILDFLAG(IS_IOS) || BUILDFLAG(IS_ANDROID)) | ||
|
|
||
| TEST(BreakIteratorTest, BreakWordChinese) { | ||
| TEST(BreakIteratorTest, DISABLED_BreakWordChinese) { |
There was a problem hiding this comment.
Instead of these, why not || defined(STARBOARD) on the #if above that encloses these three tests.
There was a problem hiding this comment.
There are actually some discussions happening here: #2074. The original idea here is to use DISABLED_ is to better tracking tests. But I don't think that point stands anymore. There will be tests that need to use || defined(STARBOARD) to disable parts of it, because they can't be compiled. I think we should have this questions discussed in meetings to keep everyone in the same page.
| DCHECK(result); | ||
| } | ||
|
|
||
| // bool LockImpl::Try() { |
There was a problem hiding this comment.
Delete commented out obsolete code
|
Btw I'm getting a little bit confused on how are we using this branch feature/update-base vs feature/all-upstream-update. Is this just purely for reviewing all past code change in feature/all-upstream-update? |
More or less, yeah, I'm thinking of feature/all-upstream-update as a place where we can make more hacky changes that would need to be cleaned up before being merged into trunk, and feature/update-base as a staging ground to clean up those changes and prepare for the final merge into trunk |
|
Is this obsolete / can close ? |
|
Yes this is obsolete |
b/298237511