Thanks for highlighting the issues. I wasn't aware of 2 at all.

Pasting https://chromium-review.googlesource.com/c/chromium/src/+/907789#message-20ab9debcc5a5ac6ffca1643ecdc9c0934bc4143 verbatim:

Also, it looks like this modifies the library code heavily, without mentioning that in README.chromium or adding a .patch file (see https://src.chromium.org/viewvc/chrome/trunk/src/third_party/README.chromium.template ; linked to from https://chromium.googlesource.com/chromium/src.git/+/master/docs/adding_to_third_party.md#Add-a-README_chromium )

(The motivation for (2) is that we want to be able to update third-party libraries from upstream easily. So ideally we wouldn't need local changes and would do changes upstream, but if we do we need to clearly document them so they can be replayed after the next update)

Any progress here? Can we remove unrar from the build until this is fixed?

The feature that uses unrar isn't enabled by default or via Finch so that code isn't getting executed today, unless enabled specifically using a command line flag.

I'll come back to fixing this after a week.

It's still bad to have code compiling with -fexceptions as part of the build, since it other people will find it in codesearch.

I agree. I can add a comment next to "//build/config/compiler:exceptions" to discourage it's use and reference this bug there. That should solve this concern for the super-short term. Would that be OK?

s/it's/its

If there's a plan to move off exceptions for unrar that can be implemented in a month or so, that sounds fine.

The NextAction date has arrived: 2018-04-30

I'm starting to look at this now.

Regarding exceptions
--------------------
I am looking into whether removing exceptions is viable. The code appears to use them in multiple places and removing that might be tricky since there are no tests to ensure it doesn't break anything.

I don't know if there's a standard replacement for throw.

Regarding patches
-----------------
I still have the original source code so generating a .diff file between the code in the repo and the original code would be straightforward but it would be the reverse of what we need here. I'm going to look into generating this reverse diff somehow. Initial searching on Google hasn't revealed anything particularly useful.

Regarding exceptions
--------------------
I found a good workaround. Instead of throw, I can do this:

+#if defined(CHROMIUM_UNRAR)
+  base::Process::Current().Terminate(Code, false);
+#else
   throw Code;
+#endif  // CHROMIUM_UNRAR

Since the library only runs within the sandbox, the sandbox process dies and the calling code handles this situation in an appropriate manner.
I ran this locally and it works as expected.

thakis@ -- do you have any thoughts on this?

See: https://chromium-review.googlesource.com/#/c/chromium/src/+/1038141 for what I describe in #c13

The NextAction date has arrived: 2018-05-02

Comment 13 sounds fine to me, but I'd feel better about it if that could make it upstream behind a "UNRAR_NO_EXCEPTIONS" define or some such, so that we don't have a big downstream diff because of it.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3317f6f503e3dc4e1a29ce27542996192608fb82

commit 3317f6f503e3dc4e1a29ce27542996192608fb82
Author: Varun Khaneja <vakh@chromium.org>
Date: Thu May 03 20:46:09 2018

unrar: Replace exceptions with terminating the current process

The callee runs in a sandbox and the caller can handle the termination
of the process as an error condition.

Bug: 826743
Change-Id: I9c0cddf9319c6db8dc2a9bf825c787a890976e0c
Reviewed-on: https://chromium-review.googlesource.com/1038141
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Nathan Parker <nparker@chromium.org>
Reviewed-by: Jialiu Lin <jialiul@chromium.org>
Commit-Queue: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555860}
[modify] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/BUILD.gn
[add] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/DEPS
[modify] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/README.chromium
[modify] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/src/errhnd.cpp
[modify] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/src/model.cpp
[modify] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/src/unpack.cpp
[modify] https://crrev.com/3317f6f503e3dc4e1a29ce27542996192608fb82/third_party/unrar/src/unpack50frag.cpp

#c19 fixes the exceptions issue.

I'm still looking for a good way to generate the patch.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9f3ebcea00305663a47c0217cee260fd74c7f0b

commit f9f3ebcea00305663a47c0217cee260fd74c7f0b
Author: Nico Weber <thakis@chromium.org>
Date: Mon Nov 05 20:33:03 2018

clang/win: Remove #pragma hdrstop workaround.

clang-cl supports #pragma hdrstop as of r341963.

Bug:  505314 , 504657 ,750327,826743
Change-Id: Ic90b07114bbb886b7d8345a260863f45eef352b7
Reviewed-on: https://chromium-review.googlesource.com/c/1317607
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Varun Khaneja <vakh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605445}
[modify] https://crrev.com/f9f3ebcea00305663a47c0217cee260fd74c7f0b/third_party/unrar/src/global.cpp