1337331 - Update security/sandbox/chromium/ to Chromium stable channel version 56.0.2924.87

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, defect)

Description

[Bob Owen (:bobowen)]

Update to commit b169b9a1cc402573843e8c952af14c4e43487e91

This is currently the latest Chromium stable release.
We can move to this now that we've dropped WinXP and Vista support.

Comment 1

[Bob Owen (:bobowen)]

Getting there. Windows issue is packaging stuff for wow_helper. Need to investigate Linux one.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a83fb51686d4bc862130c7a77bf5cd78cc9e55e7

Comment 2

[Bob Owen (:bobowen)]

Issues fixed and now with some tests:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=08e838db4cea2aae500577a54f3de59e7b51ff75

Comment 3

[Bob Owen (:bobowen)]

Attachment: Part 1: Update security/sandbox/chromium/ to commit b169b9a1cc402573843e8c952af14c4e43487e91

MozReview-Commit-ID: IuIplTq4Y8u

Comment 4

[Bob Owen (:bobowen)]

Attachment: Part 2: Re-apply - Logging changes to the Chromium interception code

Carrying r=tabraldes from previous changset:
https://hg.mozilla.org/mozilla-central/rev/a05726163a79

All of the re-application of patches this time were pretty clean apart from a couple of very minor differences, so I'm going to keep the previous m-c changeset references in the modification tracking file.

MozReview-Commit-ID: 3rwmv2eQUBN

Comment 5

[Bob Owen (:bobowen)]

Attachment: Part 3: Re-apply - Update chromium's list of linux-x86-32 syscalls

Carrying r=jld from previous changset:
https://hg.mozilla.org/mozilla-central/rev/e834e810a3fa

MozReview-Commit-ID: KnrK8HisHiX

Comment 6

[Bob Owen (:bobowen)]

Attachment: Part 4: Re-apply - Change to allow network drives in sandbox rules with non-file device fix

Carrying r=aklotz from previous changeset:
https://hg.mozilla.org/mozilla-central/rev/c70d06fa5302

MozReview-Commit-ID: 1OXOAOAxofC

Comment 7

[Bob Owen (:bobowen)]

Attachment: Part 5: Re-apply - Add KEY_WOW64_64Key and KEY_WOW64_32KEY to the Chromium sandbox allowed registry read flags

Carrying r=aklotz from previous changset:
https://hg.mozilla.org/mozilla-central/rev/d24db55deb85

MozReview-Commit-ID: EkBSisBIfP5

Comment 8

[Bob Owen (:bobowen)]

Attachment: Part 6: Re-apply - Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs

Carrying r=jimm from original changeset:
https://hg.mozilla.org/mozilla-central/rev/0e6bf137521e

MozReview-Commit-ID: ExTtkUIPXH8

Comment 9

[Bob Owen (:bobowen)]

Comment on attachment 8836080 [details] [diff] [review]
Part 2: Re-apply - Logging changes to the Chromium interception code

Carrying r=tabraldes from previous changset

Comment 10

[Bob Owen (:bobowen)]

Comment on attachment 8836082 [details] [diff] [review]
Part 3: Re-apply - Update chromium's list of linux-x86-32 syscalls

Carrying r=jld from previous changset

Comment 11

[Bob Owen (:bobowen)]

Comment on attachment 8836083 [details] [diff] [review]
Part 4: Re-apply - Change to allow network drives in sandbox rules with non-file device fix

Carrying r=aklotz from previous changeset

Comment 12

[Bob Owen (:bobowen)]

Comment on attachment 8836084 [details] [diff] [review]
Part 5: Re-apply - Add KEY_WOW64_64Key and KEY_WOW64_32KEY to the Chromium sandbox allowed registry read flags

Carrying r=aklotz from previous changset

Comment 13

[Bob Owen (:bobowen)]

Comment on attachment 8836087 [details] [diff] [review]
Part 6: Re-apply - Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs

Carrying r=jimm from original changeset

Comment 14

[Bob Owen (:bobowen)]

Comment on attachment 8836076 [details] [diff] [review]
Part 1: Update security/sandbox/chromium/ to commit b169b9a1cc402573843e8c952af14c4e43487e91

I've had to stub out quite a bit this time mainly to stop having to bring in more of file_path and its dependencies, but also for some of what I believe is chromium's telemetry and profiling code.
I've also added some MOZ_CRASH statements into the functions that we don't expect to be called, so we can catch it if they get used in future updates.

glandium - would you review the wow_helper removal build changes, please. This is pretty much the first section below and then the removal of the directory from the moz.build file.

aklotz - would you review the chromium base/ and Windows changes, please. The shim code changes and sandboxBroker/Target require a closer look.

jld - would you review the chromium base/ and Linux changes, please. Again, the shim code changes require a closer look.

Apart from the straight forward chromium file updates there are the following changes.

Changes to remove wow_helper that was only needed for WinXP and Vista 64-bit:
M browser/installer/Makefile.in
M browser/installer/package-manifest.in
M python/mozbuild/mozbuild/compilation/database.py
R security/sandbox/win/wow_helper/Makefile.in
R security/sandbox/win/wow_helper/moz.build

Changes to gecko Windows related classes and build to take account of chromium/shim changes:
M security/sandbox/moz.build - remove wow_helper and other cc file changes
M security/sandbox/win/src/sandboxbroker/moz.build - shim includes now needed
M security/sandbox/win/src/sandboxbroker/sandboxBroker.cpp - SpawnTarget changes
M security/sandbox/win/src/sandboxtarget/sandboxTarget.cpp - moved functions to here to prevent dependency leakeage into Gecko
M security/sandbox/win/src/sandboxtarget/sandboxTarget.h

Changes to linux build to take account of chromium/shim changes:
M security/sandbox/linux/moz.build - threading code changes

Chromium sandbox shim changes to take account of the chromium changes:
M security/sandbox/chromium-shim/base/file_version_info_win.h - added crashes, so we'll know if these start getting used by future changes
M security/sandbox/chromium-shim/base/files/file_path.cpp - added crash, to detect future use
M security/sandbox/chromium-shim/base/gtest_prod_util.h - added inclusion guards
M security/sandbox/chromium-shim/base/logging.cpp - added use of Unused for unused variable build failures
M security/sandbox/chromium-shim/base/tracked_objects.h - added crash, to detect future use
M security/sandbox/chromium-shim/base/win/registry.h - added new default contructor requirement and changed stubbed functions to return failures
M security/sandbox/chromium-shim/base/win/sdkdecls.h - added new Windows 10 mitigation #defines
A security/sandbox/chromium-shim/base/debug/activity_tracker.h - activity tracking stubs as we don't use it
A security/sandbox/chromium-shim/base/debug/stack_trace.h - added for scoped_handle.cc debugging, which we don't need 
A security/sandbox/chromium-shim/base/files/file_util.h - looks like this was used when the file should have really just include file_path.h
A security/sandbox/chromium-shim/base/metrics/histogram_macros.h - added empty do/while macro for Chrome histograms
A security/sandbox/chromium-shim/base/scoped_native_library.h - used UniquePtr to create auto free library class
A security/sandbox/chromium-shim/base/threading/platform_thread_linux.cpp - partial implementation to prevent more incidental dependencies
A security/sandbox/chromium-shim/base/trace_event/heap_profiler_allocation_context_tracker.h - created empty stub for AllocationContectTracker code that we don't use
A security/sandbox/chromium-shim/base/win/base_features.h - build flag file that is generated by chromium build
R security/sandbox/chromium-shim/base/MissingBasicTypes.h - no longer required

Chromium base/ additions / removals:
A security/sandbox/chromium/base/containers/adapters.h - new dependency in platform_thread_internal_posix.cc
A security/sandbox/chromium/base/environment.h - new dependency in launch.h
A security/sandbox/chromium/base/memory/free_deleter.h - replacement for scoped_ptr.h
A security/sandbox/chromium/base/memory/ptr_util.h - new dependency with std::unique_ptr utils
A security/sandbox/chromium/base/memory/shared_memory.h - new dependency in process_mitigations_win32k_dispatcher.cc
A security/sandbox/chromium/base/memory/shared_memory_handle.h - new dependency in shared_memory.h
A security/sandbox/chromium/base/memory/shared_memory_handle_win.cc - implementation for shared_memory_handle.h
A security/sandbox/chromium/base/memory/shared_memory_win.cc - implementation for shared_memory.h
A security/sandbox/chromium/base/process/kill.h - new dependency in sandbox_types.h
A security/sandbox/chromium/base/process/launch.h - new dependency in sandbox_types.h
A security/sandbox/chromium/base/process/process.h - new dependency in launch.h and kill.h
A security/sandbox/chromium/base/process/process_handle_win.cc - required for shared_memory implementation
A security/sandbox/chromium/base/rand_util_win.cc - required for shared_memory implementation
A security/sandbox/chromium/base/sequence_token.h - new dependency in sequence_checker_impl.h and thread_checker_impl.h
A security/sandbox/chromium/base/task_scheduler/task_traits.h - new dependency in sequenced_worker_pool.h
A security/sandbox/chromium/base/threading/thread_local_storage.cc - new thread_local implementation
A security/sandbox/chromium/base/threading/thread_local_storage_posix.cc - new thread_local implementation
A security/sandbox/chromium/base/threading/thread_local_storage_win.cc - new thread_local implementation
A security/sandbox/chromium/base/win/current_module.h - new dependency in scoped_handle.h and profiler.cc
R security/sandbox/chromium/base/bind_internal_win.h
R security/sandbox/chromium/base/memory/scoped_ptr.h
R security/sandbox/chromium/base/move.h
R security/sandbox/chromium/base/threading/platform_thread_linux.cc
R security/sandbox/chromium/base/threading/thread_local_posix.cc
R security/sandbox/chromium/base/threading/thread_local_win.cc

Chromium Windows sandbox removals:
R security/sandbox/chromium/sandbox/win/src/Wow64.cc
R security/sandbox/chromium/sandbox/win/src/Wow64.h
R security/sandbox/chromium/sandbox/win/src/Wow64_64.cc
R security/sandbox/chromium/sandbox/win/src/app_container.cc
R security/sandbox/chromium/sandbox/win/src/app_container.h
R security/sandbox/chromium/sandbox/win/src/app_container_unittest.cc
R security/sandbox/chromium/sandbox/win/wow_helper/service64_resolver.cc
R security/sandbox/chromium/sandbox/win/wow_helper/service64_resolver.h
R security/sandbox/chromium/sandbox/win/wow_helper/target_code.cc
R security/sandbox/chromium/sandbox/win/wow_helper/target_code.h
R security/sandbox/chromium/sandbox/win/wow_helper/wow_helper.cc

Comment 15

[Mike Hommey [:glandium]]

(In reply to Bob Owen (:bobowen) from comment #14)
> glandium - would you review the wow_helper removal build changes, please.
> This is pretty much the first section below and then the removal of the
> directory from the moz.build file.

Can I ask that you separate that out in its own patch (or even bug)? It seems to me removing wow_helper is not really tied to upgrading the chromium code.

Comment 16

[Bob Owen (:bobowen)]

(In reply to Mike Hommey [:glandium] from comment #15)
> (In reply to Bob Owen (:bobowen) from comment #14)
> > glandium - would you review the wow_helper removal build changes, please.
> > This is pretty much the first section below and then the removal of the
> > directory from the moz.build file.
> 
> Can I ask that you separate that out in its own patch (or even bug)? It
> seems to me removing wow_helper is not really tied to upgrading the chromium
> code.

The code was removed when I updated the source and that broke the packaging side of things, but you're right I can remove the wow_helper code first and avoid that.
Filed bug 1339729, patch coming soon.

Comment 17

[Bob Owen (:bobowen)]

Attachment: Part 1: Update security/sandbox/chromium/ to commit b169b9a1cc402573843e8c952af14c4e43487e91

Chromium sandbox update patch with wow_helper parts removed.
See comment 14 for details of other changes.

MozReview-Commit-ID: 5yBUVoIRNv2

Comment 18

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Bob Owen (:bobowen) (PTO until 25th Feb) from comment #17)
> Created attachment 8837501 [details] [diff] [review]
> Part 1: Update security/sandbox/chromium/ to commit
> b169b9a1cc402573843e8c952af14c4e43487e91
> 
> Chromium sandbox update patch with wow_helper parts removed.
> See comment 14 for details of other changes.
> 
> MozReview-Commit-ID: 5yBUVoIRNv2

In the future could you please try to separate the parts that I need to be focusing on in my review from the rubber-stamp stuff that is just a straight import from Chromium? I find this really tedious when it's one big code drop.

Comment 19

[Bob Owen (:bobowen)]

(In reply to Aaron Klotz [:aklotz] from comment #18)
> (In reply to Bob Owen (:bobowen) (PTO until 25th Feb) from comment #17)
> > Created attachment 8837501 [details] [diff] [review]
> > Part 1: Update security/sandbox/chromium/ to commit
> > b169b9a1cc402573843e8c952af14c4e43487e91
> > 
> > Chromium sandbox update patch with wow_helper parts removed.
> > See comment 14 for details of other changes.
> > 
> > MozReview-Commit-ID: 5yBUVoIRNv2
> 
> In the future could you please try to separate the parts that I need to be
> focusing on in my review from the rubber-stamp stuff that is just a straight
> import from Chromium? I find this really tedious when it's one big code drop.

Yes, sorry, keeping it all together is easiest when I'm creating it and of course required for it to compile.

However, I could definitely split this out (similar to the sections in my notes) before uploading and then squash back together again before pushing.
I'll do that next time.

Comment 20

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Comment on attachment 8837501 [details] [diff] [review]
Part 1: Update security/sandbox/chromium/ to commit b169b9a1cc402573843e8c952af14c4e43487e91

Review of attachment 8837501 [details] [diff] [review]:
-----------------------------------------------------------------

I got a chance to look at the chromium/sandbox/linux changes before I got hit by a car; those looked okay.

For the reams of C++11ification in base/… I was having enough trouble skimming through those when I *wasn't* in pain and under the influence of opioid painkillers, so you're on your own there.

The platform_thread_linux shim looks okay at a glance but I only looked at that just now, so see previous comment about mental state; I'm a little curious why we need GetCurrentThreadPriorityForPlatform at all but not enough to check.

Comment 21

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 8837501 [details] [diff] [review]
Part 1: Update security/sandbox/chromium/ to commit b169b9a1cc402573843e8c952af14c4e43487e91

Review of attachment 8837501 [details] [diff] [review]:
-----------------------------------------------------------------

Again, apologies for the delay. r=me with comment addressed if necessary.

::: security/sandbox/chromium-shim/base/scoped_native_library.h
@@ +23,5 @@
> +    ::FreeLibrary(hModule);
> +  }
> +};
> +
> +typedef mozilla::UniquePtr<HMODULE, HModuleFreePolicy> ScopedNativeLibrary;

Does this build correctly? In past attempts to use UniquePtr with HANDLEs I've had to wrap the HANLLE with mozilla::RemovePointer, like this:

typedef mozilla::UniquePtr<mozilla::RemovePointer<HMODULE>::Type, HModuleFreePolicy> ScopedNativeLibrary;

(because HANDLEs and such are already typedef'd as void*)

Anyway, if it builds, it builds, but if not, this might need such wrapping.

Comment 22

[Bob Owen (:bobowen)]

Thanks both for the reviews.

(In reply to Aaron Klotz [:aklotz] from comment #21)
> Comment on attachment 8837501 [details] [diff] [review]
> Part 1: Update security/sandbox/chromium/ to commit
> b169b9a1cc402573843e8c952af14c4e43487e91

> > +typedef mozilla::UniquePtr<HMODULE, HModuleFreePolicy> ScopedNativeLibrary;
> 
> Does this build correctly? In past attempts to use UniquePtr with HANDLEs
> I've had to wrap the HANLLE with mozilla::RemovePointer, like this:
> 
> typedef mozilla::UniquePtr<mozilla::RemovePointer<HMODULE>::Type,
> HModuleFreePolicy> ScopedNativeLibrary;
> 
> (because HANDLEs and such are already typedef'd as void*)
> 
> Anyway, if it builds, it builds, but if not, this might need such wrapping.

Yes, I had similar problems before I found this.

This works because of the |typedef HMODULE pointer;| line in the deleter.
It then uses this as the type instead of a pointer to whatever you specify, and so you don't have the problem of trying to specify UniquePtr<void,...>, which as you say doesn't work.

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d9872fdd25f8
Part 1: Update security/sandbox/chromium/ to commit b169b9a1cc402573843e8c952af14c4e43487e91. r=jld, r=aklotz
https://hg.mozilla.org/integration/mozilla-inbound/rev/226c893c5d62
Part 2: Re-apply - Logging changes to the Chromium interception code. r=tabraldes
https://hg.mozilla.org/integration/mozilla-inbound/rev/438b6307c802
Part 3: Re-apply - Update chromium's list of linux-x86-32 syscalls. r=jld
https://hg.mozilla.org/integration/mozilla-inbound/rev/c4aa6b85411d
Part 4: Re-apply - Change to allow network drives in sandbox rules with non-file device fix. r=aklotz
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cd2e692ee0c
Part 5: Re-apply - Add KEY_WOW64_64Key and KEY_WOW64_32KEY to the Chromium sandbox allowed registry read flags. r=aklotz
https://hg.mozilla.org/integration/mozilla-inbound/rev/0dd9bae0b6b1
Part 6: Re-apply - Change USER_NON_ADMIN access token level from whitelist to blacklist containing Admin SIDs. r=jimm

Comment 24

[Bas Schouten (:bas.schouten)]

This broke local builds for me, it appears in my build environment PROCESS_MITIGATION_FONT_DISABLE_POLICY is not defined.

Comment 25

[Bob Owen (:bobowen)]

Attachment: Part 7: Follow-up to define PROCESS_MITIGATION_FONT_DISABLE_POLICY when compiling with pre Windows 10 kit

Sorry about that, PROCESS_MITIGATION_FONT_DISABLE_POLICY is defined in the Windows 10 winnt.h file, but not in any sort of guard so I missed it.

Does this patch fix your build or are there other problems?
Not sure if the |#if !defined(PRODUCT_HOLOGRAPHIC)| trick is an acceptable hack for this sort of thing.

MozReview-Commit-ID: DzuLq5ylm78

Comment 26

[Bas Schouten (:bas.schouten)]

(In reply to Bob Owen (:bobowen) from comment #25)
> Created attachment 8850669 [details] [diff] [review]
> Part 7: Follow-up to define PROCESS_MITIGATION_FONT_DISABLE_POLICY when
> compiling with pre Windows 10 kit
> 
> Sorry about that, PROCESS_MITIGATION_FONT_DISABLE_POLICY is defined in the
> Windows 10 winnt.h file, but not in any sort of guard so I missed it.
> 
> Does this patch fix your build or are there other problems?
> Not sure if the |#if !defined(PRODUCT_HOLOGRAPHIC)| trick is an acceptable
> hack for this sort of thing.
> 
> MozReview-Commit-ID: DzuLq5ylm78

I'll check later :-). I hacked my Mozilla build to point at my Win 10 kit since it seems to pick 8.1 by default.

Comment 27

[Bas Schouten (:bas.schouten)]

Comment on attachment 8850669 [details] [diff] [review]
Part 7: Follow-up to define PROCESS_MITIGATION_FONT_DISABLE_POLICY when compiling with pre Windows 10 kit

Review of attachment 8850669 [details] [diff] [review]:
-----------------------------------------------------------------

Is it not possible to just check some TARGETSDKVER define or something along those lines?

Comment 28

[Bob Owen (:bobowen)]

Attachment: Part 7: Follow-up to define PROCESS_MITIGATION_FONT_DISABLE_POLICY when compiling with pre Windows 10 kit

/me will have to try and sneak in PRODUCT_HOLOGRAPHIC somewhere else.

MozReview-Commit-ID: DzuLq5ylm78

Comment 29

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/d9872fdd25f8
https://hg.mozilla.org/mozilla-central/rev/226c893c5d62
https://hg.mozilla.org/mozilla-central/rev/438b6307c802
https://hg.mozilla.org/mozilla-central/rev/c4aa6b85411d
https://hg.mozilla.org/mozilla-central/rev/5cd2e692ee0c
https://hg.mozilla.org/mozilla-central/rev/0dd9bae0b6b1

Comment 30

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Attachment 8850680 [details] [diff] doesn't fix the bustage for me. I get:

c:/moz/src1/security/sandbox/chromium/sandbox/win/src/process_mitigations.cc(160): error C2065: 'ProcessFontDisablePolicy': undeclared identifier

And a few other similar errors.

Comment 31

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/3f905f200fd1
Backed out changeset 0dd9bae0b6b1 
https://hg.mozilla.org/mozilla-central/rev/2aa353eee77c
Backed out changeset 5cd2e692ee0c 
https://hg.mozilla.org/mozilla-central/rev/acd487f15cce
Backed out changeset c4aa6b85411d 
https://hg.mozilla.org/mozilla-central/rev/7009e901be93
Backed out changeset 438b6307c802 
https://hg.mozilla.org/mozilla-central/rev/90bc9728e5af
Backed out changeset 226c893c5d62 
https://hg.mozilla.org/mozilla-central/rev/72bc265f157f
Backed out changeset d9872fdd25f8 for causing build problems for others + on request on bob

Comment 32

[Bill Gianopoulos [:WG9s]]

So the issue here seems to be that is makes no difference what version of the Windows 10 SDK you have installed, becuase the current version of Mozilla Build is hardcoded to use the Windows 8.1 SDK.  Therfeore, I am really puzzled as to why this works for you and why it works in automation.

Comment 33

[Bob Owen (:bobowen)]

Until we can sort out the issues with getting the Win 10 SDK picked up correctly, I think the simplest solution is just to #if out the small piece of code that is causing the issues, as we don't actually use it yet.
Just testing this works with a freshly set-up VM now.

Comment 34

[Bob Owen (:bobowen)]

Attachment: Part 1a: Follow-up to remove mitigations that require Windows 10 SDK - to be folded into Part 1

We discussed this yesterday, I can't define the missing parts, because some of them are new items in an enum.
However, the issues come from just this small part of the code that we don't use yet, so the simplest solution is just to #if it out.

I'll fold this patch into part 1 before relanding because otherwise we'll end up with a tree that doesn't compile at that changeset.

MozReview-Commit-ID: HwqM4noIHmy

Comment 35

[Jim Mathies [:jimm]]

Comment on attachment 8851993 [details] [diff] [review]
Part 1a: Follow-up to remove mitigations that require Windows 10 SDK - to be folded into Part 1

Review of attachment 8851993 [details] [diff] [review]:
-----------------------------------------------------------------

What will MITIGATION_NONSYSTEM_FONT_DISABLE give us eventually?

Comment 36

[Alex Gaynor [:Alex_Gaynor]]

The header indicates "// Prevents the process from loading non-system fonts into GDI." "PROCESS_CREATION_MITIGATION_POLICY_FONT_DISABLE_ALWAYS_ON" is the corresponding flag in the Windows SDK, but it doesn't appear to be super well documented.

Comment 37

[Bob Owen (:bobowen)]

Attachment: Part 7: Re-apply - Allow a special all paths rule in the Windows process sandbox when using semantics FILES_ALLOW_READONLY

This also changes the read only related status checks in filesystem_interception.cc to include STATUS_NETWORK_OPEN_RESTRICTION (0xC0000201), which gets returned in some cases and fails because we never ask the broker.

Carrying r=jimm from original changeset:
https://hg.mozilla.org/mozilla-central/rev/1755a454e2de

MozReview-Commit-ID: 4tfygPiKG9Z

Comment 38

[Bob Owen (:bobowen)]

Comment on attachment 8852365 [details] [diff] [review]
Part 7: Re-apply - Allow a special all paths rule in the Windows process sandbox when using semantics FILES_ALLOW_READONLY

Carrying r=jimm from original changeset.

This is something that I landed in the meantime.

Here's a Windows try push as the rebasing only affected Windows:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c6a77b3e6ce2163e1d45f04aa237f3da092fd0d

and another from the last rebasing that I don't think I added to the bug:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cd7ff2ba6de9aebe7a892282e1c258cb7a34d19c

Comment 39

[Daniel Holbert [:dholbert]]

Looks like this landed (but I think pulsebot was offline, so it wasn't reported here):

https://hg.mozilla.org/integration/mozilla-inbound/rev/e353665b9f69
https://hg.mozilla.org/integration/mozilla-inbound/rev/9af0d458ebcc
https://hg.mozilla.org/integration/mozilla-inbound/rev/1a8617171466
https://hg.mozilla.org/integration/mozilla-inbound/rev/f82b28e1b886
https://hg.mozilla.org/integration/mozilla-inbound/rev/d81b45bce315
https://hg.mozilla.org/integration/mozilla-inbound/rev/95dc42cbd89e
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e888647393a


However, I think maybe it needs a clobber to build successfully!  At least, I hit the following build error this morning when I did "hg pull -u; ./mach build" in my inbound clone:
===========
 6:25.68 libmedia_libcubeb_gtest.a.desc
 6:26.40 libmedia_psshparser_gtest.a.desc
 6:26.48 libxul-gtest.a.desc
 6:26.66 libicu.a.desc
 6:31.14 libnetwerk_dns.a.desc
 6:32.06 make[5]: *** No rule to make target '/scratch/work/builds/mozilla-inbound/mozilla/security/sandbox/chromium/base/threading/platform_thread_linux.cc', needed by 'platform_thread_linux.o'.  Stop.
 6:32.06 /scratch/work/builds/mozilla-inbound/mozilla/config/recurse.mk:71: recipe for target 'security/sandbox/linux/target' failed
 6:32.06 make[4]: *** [security/sandbox/linux/target] Error 2
 6:32.07 /scratch/work/builds/mozilla-inbound/mozilla/config/recurse.mk:32: recipe for target 'compile' failed
 6:32.07 make[3]: *** [compile] Error 2
===========

I was able to build successfully after I clobbered, though...

Comment 40

[Daniel Holbert [:dholbert]]

It looks like "part 1" here deleted the file that my build is complaining about, platform_thread_linux.cc -- quoting that patch:
> deleted file mode 100644
> --- a/security/sandbox/chromium/base/threading/platform_thread_linux.cc
> +++ /dev/null
> @@ -1,120 +0,0 @@
> -// Copyright (c) 2012 The Chromium Authors. All rights reserved.
> -// Use of this source code is governed by a BSD-style license that can be
> -// found in the LICENSE file.

There must be some cruft left over in the objdir (from a build compiled before these changes) that makes it think this file is still needed, which needs to be clobbered away.  In other words, we should land a tweak to the CLOBBER file as a followup for this bug.

Comment 41

[Bob Owen (:bobowen)]

Follow-up clobber to fix build issue from comment 39 and 40:
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/1fbba523a913d69025a63e456b5d422aac1d43a0

Comment 42

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/e353665b9f69
https://hg.mozilla.org/mozilla-central/rev/9af0d458ebcc
https://hg.mozilla.org/mozilla-central/rev/1a8617171466
https://hg.mozilla.org/mozilla-central/rev/f82b28e1b886
https://hg.mozilla.org/mozilla-central/rev/d81b45bce315
https://hg.mozilla.org/mozilla-central/rev/95dc42cbd89e
https://hg.mozilla.org/mozilla-central/rev/2e888647393a
https://hg.mozilla.org/mozilla-central/rev/1fbba523a913