Merge "Revert "Revert "Revert "Revert "Add BacktraceOffline for offline backtracing."""""
diff --git a/adb/__init__.py b/adb/__init__.py
deleted file mode 100644
index 6b509c6..0000000
--- a/adb/__init__.py
+++ /dev/null
@@ -1,17 +0,0 @@
-#
-# Copyright (C) 2015 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-from __future__ import absolute_import
-from .device import *  # pylint: disable=wildcard-import
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 60966d6..19dd03d 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -334,7 +334,7 @@
 #endif
 
     connection_properties.push_back(android::base::StringPrintf(
-        "features=%s", android::base::Join(supported_features(), ',').c_str()));
+        "features=%s", FeatureSetToString(supported_features()).c_str()));
 
     return android::base::StringPrintf(
         "%s::%s", adb_device_banner,
@@ -396,9 +396,7 @@
             } else if (key == "ro.product.device") {
                 qual_overwrite(&t->device, value);
             } else if (key == "features") {
-                for (const auto& feature : android::base::Split(value, ",")) {
-                    t->add_feature(feature);
-                }
+                t->SetFeatures(value);
             }
         }
     }
@@ -1149,27 +1147,13 @@
         std::string error_msg;
         atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg);
         if (t != nullptr) {
-            SendOkay(reply_fd, android::base::Join(t->features(), '\n'));
+            SendOkay(reply_fd, FeatureSetToString(t->features()));
         } else {
             SendFail(reply_fd, error_msg);
         }
         return 0;
     }
 
-    if (!strncmp(service, "check-feature:", strlen("check-feature:"))) {
-        std::string error_msg;
-        atransport* t = acquire_one_transport(kCsAny, type, serial, &error_msg);
-        if (t && t->CanUseFeature(service + strlen("check-feature:"))) {
-            // We could potentially extend this to reply with the feature
-            // version if that becomes necessary.
-            SendOkay(reply_fd, "1");
-        } else {
-            // Empty response means unsupported feature.
-            SendOkay(reply_fd, "");
-        }
-        return 0;
-    }
-
     // remove TCP transport
     if (!strncmp(service, "disconnect:", 11)) {
         const std::string address(service + 11);
diff --git a/adb/adb_trace.h b/adb/adb_trace.h
index 623108e..b4155df 100644
--- a/adb/adb_trace.h
+++ b/adb/adb_trace.h
@@ -50,9 +50,7 @@
 #define D(...) \
         do { \
             if (ADB_TRACING) { \
-                int saved_errno = errno; \
                 LOG(INFO) << android::base::StringPrintf(__VA_ARGS__); \
-                errno = saved_errno; \
            } \
         } while (0)
 
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index d287480..4e93dee 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -36,6 +36,7 @@
 
 #include <base/logging.h>
 #include <base/stringprintf.h>
+#include <base/strings.h>
 
 #if !defined(_WIN32)
 #include <termios.h>
@@ -108,7 +109,9 @@
         "  adb sync [ <directory> ]     - copy host->device only if changed\n"
         "                                 (-l means list but don't copy)\n"
         "  adb shell                    - run remote shell interactively\n"
-        "  adb shell <command>          - run remote shell command\n"
+        "  adb shell [-Tt] <command>    - run remote shell command\n"
+        "                                 (-T disables PTY allocation)\n"
+        "                                 (-t forces PTY allocation)\n"
         "  adb emu <command>            - run emulator console command\n"
         "  adb logcat [ <filter-spec> ] - View device log\n"
         "  adb forward --list           - list all forward socket connections.\n"
@@ -475,9 +478,10 @@
     return nullptr;
 }
 
-static int interactive_shell(bool use_shell_protocol) {
+static int interactive_shell(const std::string& service_string,
+                             bool use_shell_protocol) {
     std::string error;
-    int fd = adb_connect("shell:", &error);
+    int fd = adb_connect(service_string, &error);
     if (fd < 0) {
         fprintf(stderr,"error: %s\n", error.c_str());
         return 1;
@@ -524,18 +528,16 @@
     return android::base::StringPrintf("%s:%s", prefix, command);
 }
 
-// Checks whether the device indicated by |transport_type| and |serial| supports
-// |feature|. Returns the response string, which will be empty if the device
-// could not be found or the feature is not supported.
-static std::string CheckFeature(const std::string& feature,
-                                TransportType transport_type,
+// Returns the FeatureSet for the indicated transport.
+static FeatureSet GetFeatureSet(TransportType transport_type,
                                 const char* serial) {
-    std::string result, error, command("check-feature:" + feature);
-    if (!adb_query(format_host_command(command.c_str(), transport_type, serial),
-                   &result, &error)) {
-        return "";
+    std::string result, error;
+
+    if (adb_query(format_host_command("features", transport_type, serial),
+                  &result, &error)) {
+        return StringToFeatureSet(result);
     }
-    return result;
+    return FeatureSet();
 }
 
 static int adb_download_buffer(const char *service, const char *fn, const void* data, unsigned sz,
@@ -797,9 +799,8 @@
         wait_for_device("wait-for-device", transport_type, serial);
     }
 
-    bool use_shell_protocol = !CheckFeature(kFeatureShell2, transport_type,
-                                            serial).empty();
-    int exit_code = read_and_dump(fd, use_shell_protocol);
+    FeatureSet features = GetFeatureSet(transport_type, serial);
+    int exit_code = read_and_dump(fd, features.count(kFeatureShell2) > 0);
 
     if (adb_close(fd) < 0) {
         PLOG(ERROR) << "failure closing FD " << fd;
@@ -1238,24 +1239,44 @@
     else if (!strcmp(argv[0], "shell") || !strcmp(argv[0], "hell")) {
         char h = (argv[0][0] == 'h');
 
+        FeatureSet features = GetFeatureSet(transport_type, serial);
+
+        bool use_shell_protocol = (features.count(kFeatureShell2) > 0);
+        if (!use_shell_protocol) {
+            D("shell protocol not supported, using raw data transfer");
+        } else {
+            D("using shell protocol");
+        }
+
+        // Parse shell-specific command-line options.
+        // argv[0] is always "shell".
+        --argc;
+        ++argv;
+        std::string shell_type_arg;
+        while (argc) {
+            if (!strcmp(argv[0], "-T") || !strcmp(argv[0], "-t")) {
+                if (features.count(kFeatureShell2) == 0) {
+                    fprintf(stderr, "error: target doesn't support PTY args -Tt\n");
+                    return 1;
+                }
+                shell_type_arg = argv[0];
+                --argc;
+                ++argv;
+            } else {
+                break;
+            }
+        }
+        std::string service_string = android::base::StringPrintf(
+                "shell%s:", shell_type_arg.c_str());
+
         if (h) {
             printf("\x1b[41;33m");
             fflush(stdout);
         }
 
-        bool use_shell_protocol;
-        if (CheckFeature(kFeatureShell2, transport_type, serial).empty()) {
-            D("shell protocol not supported, using raw data transfer");
-            use_shell_protocol = false;
-        } else {
-            D("using shell protocol");
-            use_shell_protocol = true;
-        }
-
-
-        if (argc < 2) {
+        if (!argc) {
             D("starting interactive shell");
-            r = interactive_shell(use_shell_protocol);
+            r = interactive_shell(service_string, use_shell_protocol);
             if (h) {
                 printf("\x1b[0m");
                 fflush(stdout);
@@ -1263,19 +1284,14 @@
             return r;
         }
 
-        std::string cmd = "shell:";
-        --argc;
-        ++argv;
-        while (argc-- > 0) {
-            // We don't escape here, just like ssh(1). http://b/20564385.
-            cmd += *argv++;
-            if (*argv) cmd += " ";
-        }
+        // We don't escape here, just like ssh(1). http://b/20564385.
+        service_string += android::base::Join(
+                std::vector<const char*>(argv, argv + argc), ' ');
 
         while (true) {
-            D("non-interactive shell loop. cmd=%s", cmd.c_str());
+            D("non-interactive shell loop. cmd=%s", service_string.c_str());
             std::string error;
-            int fd = adb_connect(cmd, &error);
+            int fd = adb_connect(service_string, &error);
             int r;
             if (fd >= 0) {
                 D("about to read_and_dump(fd=%d)", fd);
@@ -1545,7 +1561,14 @@
         return 0;
     }
     else if (!strcmp(argv[0], "features")) {
-        return adb_query_command(format_host_command("features", transport_type, serial));
+        // Only list the features common to both the adb client and the device.
+        FeatureSet features = GetFeatureSet(transport_type, serial);
+        for (const std::string& name : features) {
+            if (supported_features().count(name) > 0) {
+                printf("%s\n", name.c_str());
+            }
+        }
+        return 0;
     }
 
     usage();
diff --git a/adb/device.py b/adb/device.py
deleted file mode 100644
index 516e880..0000000
--- a/adb/device.py
+++ /dev/null
@@ -1,339 +0,0 @@
-#
-# Copyright (C) 2015 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-import logging
-import os
-import re
-import subprocess
-import tempfile
-
-
-class FindDeviceError(RuntimeError):
-    pass
-
-
-class DeviceNotFoundError(FindDeviceError):
-    def __init__(self, serial):
-        self.serial = serial
-        super(DeviceNotFoundError, self).__init__(
-            'No device with serial {}'.format(serial))
-
-
-class NoUniqueDeviceError(FindDeviceError):
-    def __init__(self):
-        super(NoUniqueDeviceError, self).__init__('No unique device')
-
-
-class ShellError(RuntimeError):
-    def __init__(self, cmd, stdout, stderr, exit_code):
-        super(ShellError, self).__init__(
-                '`{0}` exited with code {1}'.format(cmd, exit_code))
-        self.cmd = cmd
-        self.stdout = stdout
-        self.stderr = stderr
-        self.exit_code = exit_code
-
-
-def get_devices():
-    with open(os.devnull, 'wb') as devnull:
-        subprocess.check_call(['adb', 'start-server'], stdout=devnull,
-                              stderr=devnull)
-    out = subprocess.check_output(['adb', 'devices']).splitlines()
-
-    # The first line of `adb devices` just says "List of attached devices", so
-    # skip that.
-    devices = []
-    for line in out[1:]:
-        if not line.strip():
-            continue
-        if 'offline' in line:
-            continue
-
-        serial, _ = re.split(r'\s+', line, maxsplit=1)
-        devices.append(serial)
-    return devices
-
-
-def _get_unique_device(product=None):
-    devices = get_devices()
-    if len(devices) != 1:
-        raise NoUniqueDeviceError()
-    return AndroidDevice(devices[0], product)
-
-
-def _get_device_by_serial(serial, product=None):
-    for device in get_devices():
-        if device == serial:
-            return AndroidDevice(serial, product)
-    raise DeviceNotFoundError(serial)
-
-
-def get_device(serial=None, product=None):
-    """Get a uniquely identified AndroidDevice if one is available.
-
-    Raises:
-        DeviceNotFoundError:
-            The serial specified by `serial` or $ANDROID_SERIAL is not
-            connected.
-
-        NoUniqueDeviceError:
-            Neither `serial` nor $ANDROID_SERIAL was set, and the number of
-            devices connected to the system is not 1. Having 0 connected
-            devices will also result in this error.
-
-    Returns:
-        An AndroidDevice associated with the first non-None identifier in the
-        following order of preference:
-
-        1) The `serial` argument.
-        2) The environment variable $ANDROID_SERIAL.
-        3) The single device connnected to the system.
-    """
-    if serial is not None:
-        return _get_device_by_serial(serial, product)
-
-    android_serial = os.getenv('ANDROID_SERIAL')
-    if android_serial is not None:
-        return _get_device_by_serial(android_serial, product)
-
-    return _get_unique_device(product)
-
-# Call this instead of subprocess.check_output() to work-around issue in Python
-# 2's subprocess class on Windows where it doesn't support Unicode. This
-# writes the command line to a UTF-8 batch file that is properly interpreted
-# by cmd.exe.
-def _subprocess_check_output(*popenargs, **kwargs):
-    # Only do this slow work-around if Unicode is in the cmd line.
-    if (os.name == 'nt' and
-            any(isinstance(arg, unicode) for arg in popenargs[0])):
-        # cmd.exe requires a suffix to know that it is running a batch file
-        tf = tempfile.NamedTemporaryFile('wb', suffix='.cmd', delete=False)
-        # @ in batch suppresses echo of the current line.
-        # Change the codepage to 65001, the UTF-8 codepage.
-        tf.write('@chcp 65001 > nul\r\n')
-        tf.write('@')
-        # Properly quote all the arguments and encode in UTF-8.
-        tf.write(subprocess.list2cmdline(popenargs[0]).encode('utf-8'))
-        tf.close()
-
-        try:
-            result = subprocess.check_output(['cmd.exe', '/c', tf.name],
-                                             **kwargs)
-        except subprocess.CalledProcessError as e:
-            # Show real command line instead of the cmd.exe command line.
-            raise subprocess.CalledProcessError(e.returncode, popenargs[0],
-                                                output=e.output)
-        finally:
-            os.remove(tf.name)
-        return result
-    else:
-        return subprocess.check_output(*popenargs, **kwargs)
-
-class AndroidDevice(object):
-    # Delimiter string to indicate the start of the exit code.
-    _RETURN_CODE_DELIMITER = 'x'
-
-    # Follow any shell command with this string to get the exit
-    # status of a program since this isn't propagated by adb.
-    #
-    # The delimiter is needed because `printf 1; echo $?` would print
-    # "10", and we wouldn't be able to distinguish the exit code.
-    _RETURN_CODE_PROBE_STRING = 'echo "{0}$?"'.format(_RETURN_CODE_DELIMITER)
-
-    # Maximum search distance from the output end to find the delimiter.
-    # adb on Windows returns \r\n even if adbd returns \n.
-    _RETURN_CODE_SEARCH_LENGTH = len('{0}255\r\n'.format(_RETURN_CODE_DELIMITER))
-
-    # Shell protocol feature string.
-    SHELL_PROTOCOL_FEATURE = 'shell_2'
-
-    def __init__(self, serial, product=None):
-        self.serial = serial
-        self.product = product
-        self.adb_cmd = ['adb']
-        if self.serial is not None:
-            self.adb_cmd.extend(['-s', serial])
-        if self.product is not None:
-            self.adb_cmd.extend(['-p', product])
-        self._linesep = None
-        self._features = None
-
-    @property
-    def linesep(self):
-        if self._linesep is None:
-            self._linesep = subprocess.check_output(self.adb_cmd +
-                                                    ['shell', 'echo'])
-        return self._linesep
-
-    @property
-    def features(self):
-        if self._features is None:
-            try:
-                self._features = self._simple_call(['features']).splitlines()
-            except subprocess.CalledProcessError:
-                self._features = []
-        return self._features
-
-    def _make_shell_cmd(self, user_cmd):
-        command = self.adb_cmd + ['shell'] + user_cmd
-        if self.SHELL_PROTOCOL_FEATURE not in self.features:
-            command.append('; ' + self._RETURN_CODE_PROBE_STRING)
-        return command
-
-    def _parse_shell_output(self, out):
-        """Finds the exit code string from shell output.
-
-        Args:
-            out: Shell output string.
-
-        Returns:
-            An (exit_code, output_string) tuple. The output string is
-            cleaned of any additional stuff we appended to find the
-            exit code.
-
-        Raises:
-            RuntimeError: Could not find the exit code in |out|.
-        """
-        search_text = out
-        if len(search_text) > self._RETURN_CODE_SEARCH_LENGTH:
-            # We don't want to search over massive amounts of data when we know
-            # the part we want is right at the end.
-            search_text = search_text[-self._RETURN_CODE_SEARCH_LENGTH:]
-        partition = search_text.rpartition(self._RETURN_CODE_DELIMITER)
-        if partition[1] == '':
-            raise RuntimeError('Could not find exit status in shell output.')
-        result = int(partition[2])
-        # partition[0] won't contain the full text if search_text was truncated,
-        # pull from the original string instead.
-        out = out[:-len(partition[1]) - len(partition[2])]
-        return result, out
-
-    def _simple_call(self, cmd):
-        logging.info(' '.join(self.adb_cmd + cmd))
-        return _subprocess_check_output(
-            self.adb_cmd + cmd, stderr=subprocess.STDOUT)
-
-    def shell(self, cmd):
-        """Calls `adb shell`
-
-        Args:
-            cmd: string shell command to execute.
-
-        Returns:
-            A (stdout, stderr) tuple. Stderr may be combined into stdout
-            if the device doesn't support separate streams.
-
-        Raises:
-            ShellError: the exit code was non-zero.
-        """
-        exit_code, stdout, stderr = self.shell_nocheck(cmd)
-        if exit_code != 0:
-            raise ShellError(cmd, stdout, stderr, exit_code)
-        return stdout, stderr
-
-    def shell_nocheck(self, cmd):
-        """Calls `adb shell`
-
-        Args:
-            cmd: string shell command to execute.
-
-        Returns:
-            An (exit_code, stdout, stderr) tuple. Stderr may be combined
-            into stdout if the device doesn't support separate streams.
-        """
-        cmd = self._make_shell_cmd(cmd)
-        logging.info(' '.join(cmd))
-        p = subprocess.Popen(
-            cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
-        stdout, stderr = p.communicate()
-        if self.SHELL_PROTOCOL_FEATURE in self.features:
-            exit_code = p.returncode
-        else:
-            exit_code, stdout = self._parse_shell_output(stdout)
-        return exit_code, stdout, stderr
-
-    def install(self, filename, replace=False):
-        cmd = ['install']
-        if replace:
-            cmd.append('-r')
-        cmd.append(filename)
-        return self._simple_call(cmd)
-
-    def push(self, local, remote):
-        return self._simple_call(['push', local, remote])
-
-    def pull(self, remote, local):
-        return self._simple_call(['pull', remote, local])
-
-    def sync(self, directory=None):
-        cmd = ['sync']
-        if directory is not None:
-            cmd.append(directory)
-        return self._simple_call(cmd)
-
-    def forward(self, local, remote):
-        return self._simple_call(['forward', local, remote])
-
-    def tcpip(self, port):
-        return self._simple_call(['tcpip', port])
-
-    def usb(self):
-        return self._simple_call(['usb'])
-
-    def reboot(self):
-        return self._simple_call(['reboot'])
-
-    def root(self):
-        return self._simple_call(['root'])
-
-    def unroot(self):
-        return self._simple_call(['unroot'])
-
-    def forward_remove(self, local):
-        return self._simple_call(['forward', '--remove', local])
-
-    def forward_remove_all(self):
-        return self._simple_call(['forward', '--remove-all'])
-
-    def connect(self, host):
-        return self._simple_call(['connect', host])
-
-    def disconnect(self, host):
-        return self._simple_call(['disconnect', host])
-
-    def reverse(self, remote, local):
-        return self._simple_call(['reverse', remote, local])
-
-    def reverse_remove_all(self):
-        return self._simple_call(['reverse', '--remove-all'])
-
-    def reverse_remove(self, remote):
-        return self._simple_call(['reverse', '--remove', remote])
-
-    def wait(self):
-        return self._simple_call(['wait-for-device'])
-
-    def get_prop(self, prop_name):
-        output = self.shell(['getprop', prop_name])[0].splitlines()
-        if len(output) != 1:
-            raise RuntimeError('Too many lines in getprop output:\n' +
-                               '\n'.join(output))
-        value = output[0]
-        if not value.strip():
-            return None
-        return value
-
-    def set_prop(self, prop_name, value):
-        self.shell(['setprop', prop_name, value])
diff --git a/adb/services.cpp b/adb/services.cpp
index d128efc..d0494ec 100644
--- a/adb/services.cpp
+++ b/adb/services.cpp
@@ -194,7 +194,39 @@
     adb_close(fd);
 }
 
-#endif
+// Shell service string can look like:
+//   shell[args]:[command]
+// Currently the only supported args are -T (force raw) and -t (force PTY).
+static int ShellService(const std::string& args, const atransport* transport) {
+    size_t delimiter_index = args.find(':');
+    if (delimiter_index == std::string::npos) {
+        LOG(ERROR) << "No ':' found in shell service arguments: " << args;
+        return -1;
+    }
+    const std::string service_args = args.substr(0, delimiter_index);
+    const std::string command = args.substr(delimiter_index + 1);
+
+    SubprocessType type;
+    if (service_args.empty()) {
+        // Default: use PTY for interactive, raw for non-interactive.
+        type = (command.empty() ? SubprocessType::kPty : SubprocessType::kRaw);
+    } else if (service_args == "-T") {
+        type = SubprocessType::kRaw;
+    } else if (service_args == "-t") {
+        type = SubprocessType::kPty;
+    } else {
+        LOG(ERROR) << "Unsupported shell service arguments: " << args;
+        return -1;
+    }
+
+    SubprocessProtocol protocol =
+            (transport->CanUseFeature(kFeatureShell2) ? SubprocessProtocol::kShell
+                                                      : SubprocessProtocol::kNone);
+
+    return StartSubprocess(command.c_str(), type, protocol);
+}
+
+#endif  // !ADB_HOST
 
 static int create_service_thread(void (*func)(int, void *), void *cookie)
 {
@@ -265,14 +297,8 @@
         ret = create_service_thread(framebuffer_service, 0);
     } else if (!strncmp(name, "jdwp:", 5)) {
         ret = create_jdwp_connection_fd(atoi(name+5));
-    } else if(!strncmp(name, "shell:", 6)) {
-        const char* args = name + 6;
-        // Use raw for non-interactive, PTY for interactive.
-        SubprocessType type = (*args ? SubprocessType::kRaw : SubprocessType::kPty);
-        SubprocessProtocol protocol =
-                (transport->CanUseFeature(kFeatureShell2) ? SubprocessProtocol::kShell
-                                                          : SubprocessProtocol::kNone);
-        ret = StartSubprocess(args, type, protocol);
+    } else if(!strncmp(name, "shell", 5)) {
+        ret = ShellService(name + 5, transport);
     } else if(!strncmp(name, "exec:", 5)) {
         ret = StartSubprocess(name + 5, SubprocessType::kRaw,
                               SubprocessProtocol::kNone);
diff --git a/adb/test_device.py b/adb/test_device.py
deleted file mode 100644
index d033a01..0000000
--- a/adb/test_device.py
+++ /dev/null
@@ -1,537 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-#
-# Copyright (C) 2015 The Android Open Source Project
-#
-# Licensed under the Apache License, Version 2.0 (the "License");
-# you may not use this file except in compliance with the License.
-# You may obtain a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS,
-# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-# See the License for the specific language governing permissions and
-# limitations under the License.
-#
-from __future__ import print_function
-
-import hashlib
-import os
-import posixpath
-import random
-import shlex
-import shutil
-import signal
-import subprocess
-import tempfile
-import unittest
-
-import mock
-
-import adb
-
-
-def requires_root(func):
-    def wrapper(self, *args):
-        if self.device.get_prop('ro.debuggable') != '1':
-            raise unittest.SkipTest('requires rootable build')
-
-        was_root = self.device.shell(['id', '-un'])[0].strip() == 'root'
-        if not was_root:
-            self.device.root()
-            self.device.wait()
-
-        try:
-            func(self, *args)
-        finally:
-            if not was_root:
-                self.device.unroot()
-                self.device.wait()
-
-    return wrapper
-
-
-class GetDeviceTest(unittest.TestCase):
-    def setUp(self):
-        self.android_serial = os.getenv('ANDROID_SERIAL')
-        if 'ANDROID_SERIAL' in os.environ:
-            del os.environ['ANDROID_SERIAL']
-
-    def tearDown(self):
-        if self.android_serial is not None:
-            os.environ['ANDROID_SERIAL'] = self.android_serial
-        else:
-            if 'ANDROID_SERIAL' in os.environ:
-                del os.environ['ANDROID_SERIAL']
-
-    @mock.patch('adb.device.get_devices')
-    def test_explicit(self, mock_get_devices):
-        mock_get_devices.return_value = ['foo', 'bar']
-        device = adb.get_device('foo')
-        self.assertEqual(device.serial, 'foo')
-
-    @mock.patch('adb.device.get_devices')
-    def test_from_env(self, mock_get_devices):
-        mock_get_devices.return_value = ['foo', 'bar']
-        os.environ['ANDROID_SERIAL'] = 'foo'
-        device = adb.get_device()
-        self.assertEqual(device.serial, 'foo')
-
-    @mock.patch('adb.device.get_devices')
-    def test_arg_beats_env(self, mock_get_devices):
-        mock_get_devices.return_value = ['foo', 'bar']
-        os.environ['ANDROID_SERIAL'] = 'bar'
-        device = adb.get_device('foo')
-        self.assertEqual(device.serial, 'foo')
-
-    @mock.patch('adb.device.get_devices')
-    def test_no_such_device(self, mock_get_devices):
-        mock_get_devices.return_value = ['foo', 'bar']
-        self.assertRaises(adb.DeviceNotFoundError, adb.get_device, ['baz'])
-
-        os.environ['ANDROID_SERIAL'] = 'baz'
-        self.assertRaises(adb.DeviceNotFoundError, adb.get_device)
-
-    @mock.patch('adb.device.get_devices')
-    def test_unique_device(self, mock_get_devices):
-        mock_get_devices.return_value = ['foo']
-        device = adb.get_device()
-        self.assertEqual(device.serial, 'foo')
-
-    @mock.patch('adb.device.get_devices')
-    def test_no_unique_device(self, mock_get_devices):
-        mock_get_devices.return_value = ['foo', 'bar']
-        self.assertRaises(adb.NoUniqueDeviceError, adb.get_device)
-
-
-class DeviceTest(unittest.TestCase):
-    def setUp(self):
-        self.device = adb.get_device()
-
-
-class ShellTest(DeviceTest):
-    def test_cat(self):
-        """Check that we can at least cat a file."""
-        out = self.device.shell(['cat', '/proc/uptime'])[0].strip()
-        elements = out.split()
-        self.assertEqual(len(elements), 2)
-
-        uptime, idle = elements
-        self.assertGreater(float(uptime), 0.0)
-        self.assertGreater(float(idle), 0.0)
-
-    def test_throws_on_failure(self):
-        self.assertRaises(adb.ShellError, self.device.shell, ['false'])
-
-    def test_output_not_stripped(self):
-        out = self.device.shell(['echo', 'foo'])[0]
-        self.assertEqual(out, 'foo' + self.device.linesep)
-
-    def test_shell_nocheck_failure(self):
-        rc, out, _ = self.device.shell_nocheck(['false'])
-        self.assertNotEqual(rc, 0)
-        self.assertEqual(out, '')
-
-    def test_shell_nocheck_output_not_stripped(self):
-        rc, out, _ = self.device.shell_nocheck(['echo', 'foo'])
-        self.assertEqual(rc, 0)
-        self.assertEqual(out, 'foo' + self.device.linesep)
-
-    def test_can_distinguish_tricky_results(self):
-        # If result checking on ADB shell is naively implemented as
-        # `adb shell <cmd>; echo $?`, we would be unable to distinguish the
-        # output from the result for a cmd of `echo -n 1`.
-        rc, out, _ = self.device.shell_nocheck(['echo', '-n', '1'])
-        self.assertEqual(rc, 0)
-        self.assertEqual(out, '1')
-
-    def test_line_endings(self):
-        """Ensure that line ending translation is not happening in the pty.
-
-        Bug: http://b/19735063
-        """
-        output = self.device.shell(['uname'])[0]
-        self.assertEqual(output, 'Linux' + self.device.linesep)
-
-    def test_pty_logic(self):
-        """Verify PTY logic for shells.
-
-        Interactive shells should use a PTY, non-interactive should not.
-
-        Bug: http://b/21215503
-        """
-        proc = subprocess.Popen(
-                self.device.adb_cmd + ['shell'], stdin=subprocess.PIPE,
-                stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
-        # [ -t 0 ] is used (rather than `tty`) to provide portability. This
-        # gives an exit code of 0 iff stdin is connected to a terminal.
-        #
-        # Closing host-side stdin doesn't currently trigger the interactive
-        # shell to exit so we need to explicitly add an exit command to
-        # close the session from the device side, and append \n to complete
-        # the interactive command.
-        result = proc.communicate('[ -t 0 ]; echo x$?; exit 0\n')[0]
-        partition = result.rpartition('x')
-        self.assertEqual(partition[1], 'x')
-        self.assertEqual(int(partition[2]), 0)
-
-        exit_code = self.device.shell_nocheck(['[ -t 0 ]'])[0]
-        self.assertEqual(exit_code, 1)
-
-    def test_shell_protocol(self):
-        """Tests the shell protocol on the device.
-
-        If the device supports shell protocol, this gives us the ability
-        to separate stdout/stderr and return the exit code directly.
-
-        Bug: http://b/19734861
-        """
-        if self.device.SHELL_PROTOCOL_FEATURE not in self.device.features:
-            raise unittest.SkipTest('shell protocol unsupported on this device')
-        result = self.device.shell_nocheck(
-                shlex.split('echo foo; echo bar >&2; exit 17'))
-
-        self.assertEqual(17, result[0])
-        self.assertEqual('foo' + self.device.linesep, result[1])
-        self.assertEqual('bar' + self.device.linesep, result[2])
-
-    def test_non_interactive_sigint(self):
-        """Tests that SIGINT in a non-interactive shell kills the process.
-
-        This requires the shell protocol in order to detect the broken
-        pipe; raw data transfer mode will only see the break once the
-        subprocess tries to read or write.
-
-        Bug: http://b/23825725
-        """
-        if self.device.SHELL_PROTOCOL_FEATURE not in self.device.features:
-            raise unittest.SkipTest('shell protocol unsupported on this device')
-
-        # Start a long-running process.
-        sleep_proc = subprocess.Popen(
-                self.device.adb_cmd + shlex.split('shell echo $$; sleep 60'),
-                stdin=subprocess.PIPE, stdout=subprocess.PIPE,
-                stderr=subprocess.STDOUT)
-        remote_pid = sleep_proc.stdout.readline().strip()
-        self.assertIsNone(sleep_proc.returncode, 'subprocess terminated early')
-        proc_query = shlex.split('ps {0} | grep {0}'.format(remote_pid))
-
-        # Verify that the process is running, send signal, verify it stopped.
-        self.device.shell(proc_query)
-        os.kill(sleep_proc.pid, signal.SIGINT)
-        sleep_proc.communicate()
-        self.assertEqual(1, self.device.shell_nocheck(proc_query)[0],
-                         'subprocess failed to terminate')
-
-
-class ArgumentEscapingTest(DeviceTest):
-    def test_shell_escaping(self):
-        """Make sure that argument escaping is somewhat sane."""
-
-        # http://b/19734868
-        # Note that this actually matches ssh(1)'s behavior --- it's
-        # converted to `sh -c echo hello; echo world` which sh interprets
-        # as `sh -c echo` (with an argument to that shell of "hello"),
-        # and then `echo world` back in the first shell.
-        result = self.device.shell(
-            shlex.split("sh -c 'echo hello; echo world'"))[0]
-        result = result.splitlines()
-        self.assertEqual(['', 'world'], result)
-        # If you really wanted "hello" and "world", here's what you'd do:
-        result = self.device.shell(
-            shlex.split(r'echo hello\;echo world'))[0].splitlines()
-        self.assertEqual(['hello', 'world'], result)
-
-        # http://b/15479704
-        result = self.device.shell(shlex.split("'true && echo t'"))[0].strip()
-        self.assertEqual('t', result)
-        result = self.device.shell(
-            shlex.split("sh -c 'true && echo t'"))[0].strip()
-        self.assertEqual('t', result)
-
-        # http://b/20564385
-        result = self.device.shell(shlex.split('FOO=a BAR=b echo t'))[0].strip()
-        self.assertEqual('t', result)
-        result = self.device.shell(
-            shlex.split(r'echo -n 123\;uname'))[0].strip()
-        self.assertEqual('123Linux', result)
-
-    def test_install_argument_escaping(self):
-        """Make sure that install argument escaping works."""
-        # http://b/20323053
-        tf = tempfile.NamedTemporaryFile('wb', suffix='-text;ls;1.apk',
-                                         delete=False)
-        tf.close()
-        self.assertIn("-text;ls;1.apk", self.device.install(tf.name))
-        os.remove(tf.name)
-
-        # http://b/3090932
-        tf = tempfile.NamedTemporaryFile('wb', suffix="-Live Hold'em.apk",
-                                         delete=False)
-        tf.close()
-        self.assertIn("-Live Hold'em.apk", self.device.install(tf.name))
-        os.remove(tf.name)
-
-
-class RootUnrootTest(DeviceTest):
-    def _test_root(self):
-        message = self.device.root()
-        if 'adbd cannot run as root in production builds' in message:
-            return
-        self.device.wait()
-        self.assertEqual('root', self.device.shell(['id', '-un'])[0].strip())
-
-    def _test_unroot(self):
-        self.device.unroot()
-        self.device.wait()
-        self.assertEqual('shell', self.device.shell(['id', '-un'])[0].strip())
-
-    def test_root_unroot(self):
-        """Make sure that adb root and adb unroot work, using id(1)."""
-        if self.device.get_prop('ro.debuggable') != '1':
-            raise unittest.SkipTest('requires rootable build')
-
-        original_user = self.device.shell(['id', '-un'])[0].strip()
-        try:
-            if original_user == 'root':
-                self._test_unroot()
-                self._test_root()
-            elif original_user == 'shell':
-                self._test_root()
-                self._test_unroot()
-        finally:
-            if original_user == 'root':
-                self.device.root()
-            else:
-                self.device.unroot()
-            self.device.wait()
-
-
-class TcpIpTest(DeviceTest):
-    def test_tcpip_failure_raises(self):
-        """adb tcpip requires a port.
-
-        Bug: http://b/22636927
-        """
-        self.assertRaises(
-            subprocess.CalledProcessError, self.device.tcpip, '')
-        self.assertRaises(
-            subprocess.CalledProcessError, self.device.tcpip, 'foo')
-
-
-class SystemPropertiesTest(DeviceTest):
-    def test_get_prop(self):
-        self.assertEqual(self.device.get_prop('init.svc.adbd'), 'running')
-
-    @requires_root
-    def test_set_prop(self):
-        prop_name = 'foo.bar'
-        self.device.shell(['setprop', prop_name, '""'])
-
-        self.device.set_prop(prop_name, 'qux')
-        self.assertEqual(
-            self.device.shell(['getprop', prop_name])[0].strip(), 'qux')
-
-
-def compute_md5(string):
-    hsh = hashlib.md5()
-    hsh.update(string)
-    return hsh.hexdigest()
-
-
-def get_md5_prog(device):
-    """Older platforms (pre-L) had the name md5 rather than md5sum."""
-    try:
-        device.shell(['md5sum', '/proc/uptime'])
-        return 'md5sum'
-    except subprocess.CalledProcessError:
-        return 'md5'
-
-
-class HostFile(object):
-    def __init__(self, handle, checksum):
-        self.handle = handle
-        self.checksum = checksum
-        self.full_path = handle.name
-        self.base_name = os.path.basename(self.full_path)
-
-
-class DeviceFile(object):
-    def __init__(self, checksum, full_path):
-        self.checksum = checksum
-        self.full_path = full_path
-        self.base_name = posixpath.basename(self.full_path)
-
-
-def make_random_host_files(in_dir, num_files):
-    min_size = 1 * (1 << 10)
-    max_size = 16 * (1 << 10)
-
-    files = []
-    for _ in xrange(num_files):
-        file_handle = tempfile.NamedTemporaryFile(dir=in_dir, delete=False)
-
-        size = random.randrange(min_size, max_size, 1024)
-        rand_str = os.urandom(size)
-        file_handle.write(rand_str)
-        file_handle.flush()
-        file_handle.close()
-
-        md5 = compute_md5(rand_str)
-        files.append(HostFile(file_handle, md5))
-    return files
-
-
-def make_random_device_files(device, in_dir, num_files):
-    min_size = 1 * (1 << 10)
-    max_size = 16 * (1 << 10)
-
-    files = []
-    for file_num in xrange(num_files):
-        size = random.randrange(min_size, max_size, 1024)
-
-        base_name = 'device_tmpfile' + str(file_num)
-        full_path = posixpath.join(in_dir, base_name)
-
-        device.shell(['dd', 'if=/dev/urandom', 'of={}'.format(full_path),
-                      'bs={}'.format(size), 'count=1'])
-        dev_md5, _ = device.shell([get_md5_prog(device), full_path])[0].split()
-
-        files.append(DeviceFile(dev_md5, full_path))
-    return files
-
-
-class FileOperationsTest(DeviceTest):
-    SCRATCH_DIR = '/data/local/tmp'
-    DEVICE_TEMP_FILE = SCRATCH_DIR + '/adb_test_file'
-    DEVICE_TEMP_DIR = SCRATCH_DIR + '/adb_test_dir'
-
-    def _test_push(self, local_file, checksum):
-        self.device.shell(['rm', '-rf', self.DEVICE_TEMP_FILE])
-        self.device.push(local=local_file, remote=self.DEVICE_TEMP_FILE)
-        dev_md5, _ = self.device.shell([get_md5_prog(self.device),
-                                       self.DEVICE_TEMP_FILE])[0].split()
-        self.assertEqual(checksum, dev_md5)
-        self.device.shell(['rm', '-f', self.DEVICE_TEMP_FILE])
-
-    def test_push(self):
-        """Push a randomly generated file to specified device."""
-        kbytes = 512
-        tmp = tempfile.NamedTemporaryFile(mode='wb', delete=False)
-        rand_str = os.urandom(1024 * kbytes)
-        tmp.write(rand_str)
-        tmp.close()
-        self._test_push(tmp.name, compute_md5(rand_str))
-        os.remove(tmp.name)
-
-    # TODO: write push directory test.
-
-    def _test_pull(self, remote_file, checksum):
-        tmp_write = tempfile.NamedTemporaryFile(mode='wb', delete=False)
-        tmp_write.close()
-        self.device.pull(remote=remote_file, local=tmp_write.name)
-        with open(tmp_write.name, 'rb') as tmp_read:
-            host_contents = tmp_read.read()
-            host_md5 = compute_md5(host_contents)
-        self.assertEqual(checksum, host_md5)
-        os.remove(tmp_write.name)
-
-    def test_pull(self):
-        """Pull a randomly generated file from specified device."""
-        kbytes = 512
-        self.device.shell(['rm', '-rf', self.DEVICE_TEMP_FILE])
-        cmd = ['dd', 'if=/dev/urandom',
-               'of={}'.format(self.DEVICE_TEMP_FILE), 'bs=1024',
-               'count={}'.format(kbytes)]
-        self.device.shell(cmd)
-        dev_md5, _ = self.device.shell(
-            [get_md5_prog(self.device), self.DEVICE_TEMP_FILE])[0].split()
-        self._test_pull(self.DEVICE_TEMP_FILE, dev_md5)
-        self.device.shell_nocheck(['rm', self.DEVICE_TEMP_FILE])
-
-    def test_pull_dir(self):
-        """Pull a randomly generated directory of files from the device."""
-        host_dir = tempfile.mkdtemp()
-        self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
-        self.device.shell(['mkdir', '-p', self.DEVICE_TEMP_DIR])
-
-        # Populate device directory with random files.
-        temp_files = make_random_device_files(
-            self.device, in_dir=self.DEVICE_TEMP_DIR, num_files=32)
-
-        self.device.pull(remote=self.DEVICE_TEMP_DIR, local=host_dir)
-
-        for temp_file in temp_files:
-            host_path = os.path.join(host_dir, temp_file.base_name)
-            with open(host_path, 'rb') as host_file:
-                host_md5 = compute_md5(host_file.read())
-                self.assertEqual(host_md5, temp_file.checksum)
-
-        self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
-        if host_dir is not None:
-            shutil.rmtree(host_dir)
-
-    def test_sync(self):
-        """Sync a randomly generated directory of files to specified device."""
-        base_dir = tempfile.mkdtemp()
-
-        # Create mirror device directory hierarchy within base_dir.
-        full_dir_path = base_dir + self.DEVICE_TEMP_DIR
-        os.makedirs(full_dir_path)
-
-        # Create 32 random files within the host mirror.
-        temp_files = make_random_host_files(in_dir=full_dir_path, num_files=32)
-
-        # Clean up any trash on the device.
-        device = adb.get_device(product=base_dir)
-        device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
-
-        device.sync('data')
-
-        # Confirm that every file on the device mirrors that on the host.
-        for temp_file in temp_files:
-            device_full_path = posixpath.join(self.DEVICE_TEMP_DIR,
-                                              temp_file.base_name)
-            dev_md5, _ = device.shell(
-                [get_md5_prog(self.device), device_full_path])[0].split()
-            self.assertEqual(temp_file.checksum, dev_md5)
-
-        self.device.shell(['rm', '-rf', self.DEVICE_TEMP_DIR])
-        if base_dir is not None:
-            shutil.rmtree(base_dir)
-
-    def test_unicode_paths(self):
-        """Ensure that we can support non-ASCII paths, even on Windows."""
-        name = u'로보카 폴리'
-
-        ## push.
-        tf = tempfile.NamedTemporaryFile('wb', suffix=name, delete=False)
-        tf.close()
-        self.device.push(tf.name, u'/data/local/tmp/adb-test-{}'.format(name))
-        os.remove(tf.name)
-        self.device.shell(['rm', '-f', '/data/local/tmp/adb-test-*'])
-
-        # pull.
-        cmd = ['touch', u'"/data/local/tmp/adb-test-{}"'.format(name)]
-        self.device.shell(cmd)
-
-        tf = tempfile.NamedTemporaryFile('wb', suffix=name, delete=False)
-        tf.close()
-        self.device.pull(u'/data/local/tmp/adb-test-{}'.format(name), tf.name)
-        os.remove(tf.name)
-        self.device.shell(['rm', '-f', '/data/local/tmp/adb-test-*'])
-
-
-def main():
-    random.seed(0)
-    if len(adb.get_devices()) > 0:
-        suite = unittest.TestLoader().loadTestsFromName(__name__)
-        unittest.TextTestRunner(verbosity=3).run(suite)
-    else:
-        print('Test suite must be run with attached devices')
-
-
-if __name__ == '__main__':
-    main()
diff --git a/adb/transport.cpp b/adb/transport.cpp
index db9236e..402546b 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -779,27 +779,37 @@
     return max_payload;
 }
 
-// Do not use any of [:;=,] in feature strings, they have special meaning
-// in the connection banner.
-// TODO(dpursell): add this in once we can pass features through to the client.
-const char kFeatureShell2[] = "shell_2";
+namespace {
 
-// The list of features supported by the current system. Will be sent to the
-// other side of the connection in the banner.
-static const FeatureSet gSupportedFeatures = {
-        kFeatureShell2,
-};
+constexpr char kFeatureStringDelimiter = ',';
+
+}  // namespace
 
 const FeatureSet& supported_features() {
-    return gSupportedFeatures;
+    // Local static allocation to avoid global non-POD variables.
+    static const FeatureSet* features = new FeatureSet{
+        kFeatureShell2
+    };
+
+    return *features;
+}
+
+std::string FeatureSetToString(const FeatureSet& features) {
+    return android::base::Join(features, kFeatureStringDelimiter);
+}
+
+FeatureSet StringToFeatureSet(const std::string& features_string) {
+    auto names = android::base::Split(features_string,
+                                      {kFeatureStringDelimiter});
+    return FeatureSet(names.begin(), names.end());
 }
 
 bool atransport::has_feature(const std::string& feature) const {
     return features_.count(feature) > 0;
 }
 
-void atransport::add_feature(const std::string& feature) {
-    features_.insert(feature);
+void atransport::SetFeatures(const std::string& features_string) {
+    features_ = StringToFeatureSet(features_string);
 }
 
 bool atransport::CanUseFeature(const std::string& feature) const {
@@ -855,9 +865,6 @@
         append_transport_info(result, "product:", t->product, false);
         append_transport_info(result, "model:", t->model, true);
         append_transport_info(result, "device:", t->device, false);
-        append_transport_info(result, "features:",
-                              android::base::Join(t->features(), ',').c_str(),
-                              false);
     }
     *result += '\n';
 }
diff --git a/adb/transport.h b/adb/transport.h
index 999922a..0ec8ceb 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -29,7 +29,13 @@
 
 const FeatureSet& supported_features();
 
-const extern char kFeatureShell2[];
+// Encodes and decodes FeatureSet objects into human-readable strings.
+std::string FeatureSetToString(const FeatureSet& features);
+FeatureSet StringToFeatureSet(const std::string& features_string);
+
+// Do not use any of [:;=,] in feature strings, they have special meaning
+// in the connection banner.
+constexpr char kFeatureShell2[] = "shell_2";
 
 class atransport {
 public:
@@ -85,12 +91,14 @@
     int get_protocol_version() const;
     size_t get_max_payload() const;
 
-    inline const FeatureSet features() const {
+    const FeatureSet& features() const {
         return features_;
     }
 
     bool has_feature(const std::string& feature) const;
-    void add_feature(const std::string& feature);
+
+    // Loads the transport's feature set from the given string.
+    void SetFeatures(const std::string& features_string);
 
     // Returns true if both we and the other end of the transport support the
     // feature.
diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp
index 10872ac..7d69c3e 100644
--- a/adb/transport_test.cpp
+++ b/adb/transport_test.cpp
@@ -144,23 +144,29 @@
     ASSERT_EQ(0, count);
 }
 
-TEST(transport, add_feature) {
+TEST(transport, SetFeatures) {
     atransport t;
     ASSERT_EQ(0U, t.features().size());
 
-    t.add_feature("foo");
+    t.SetFeatures(FeatureSetToString(FeatureSet{"foo"}));
     ASSERT_EQ(1U, t.features().size());
     ASSERT_TRUE(t.has_feature("foo"));
 
-    t.add_feature("bar");
+    t.SetFeatures(FeatureSetToString(FeatureSet{"foo", "bar"}));
     ASSERT_EQ(2U, t.features().size());
     ASSERT_TRUE(t.has_feature("foo"));
     ASSERT_TRUE(t.has_feature("bar"));
 
-    t.add_feature("foo");
+    t.SetFeatures(FeatureSetToString(FeatureSet{"foo", "bar", "foo"}));
     ASSERT_EQ(2U, t.features().size());
     ASSERT_TRUE(t.has_feature("foo"));
     ASSERT_TRUE(t.has_feature("bar"));
+
+    t.SetFeatures(FeatureSetToString(FeatureSet{"bar", "baz"}));
+    ASSERT_EQ(2U, t.features().size());
+    ASSERT_FALSE(t.has_feature("foo"));
+    ASSERT_TRUE(t.has_feature("bar"));
+    ASSERT_TRUE(t.has_feature("baz"));
 }
 
 TEST(transport, parse_banner_no_features) {
diff --git a/base/include/base/logging.h b/base/include/base/logging.h
index 283a7bc..30f6906 100644
--- a/base/include/base/logging.h
+++ b/base/include/base/logging.h
@@ -87,30 +87,64 @@
 // Replace the current logger.
 extern void SetLogger(LogFunction&& logger);
 
+// Get the minimum severity level for logging.
+extern LogSeverity GetMinimumLogSeverity();
+
+class ErrnoRestorer {
+ public:
+  ErrnoRestorer()
+      : saved_errno_(errno) {
+  }
+
+  ~ErrnoRestorer() {
+    errno = saved_errno_;
+  }
+
+  // Allow this object to evaluate to false which is useful in macros.
+  operator bool() const {
+    return false;
+  }
+
+ private:
+  const int saved_errno_;
+
+  DISALLOW_COPY_AND_ASSIGN(ErrnoRestorer);
+};
+
 // Logs a message to logcat on Android otherwise to stderr. If the severity is
 // FATAL it also causes an abort. For example:
 //
 //     LOG(FATAL) << "We didn't expect to reach here";
-#define LOG(severity)                                                       \
-  ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::DEFAULT, \
-                              ::android::base::severity, -1).stream()
+#define LOG(severity) LOG_TO(DEFAULT, severity)
 
 // Logs a message to logcat with the specified log ID on Android otherwise to
 // stderr. If the severity is FATAL it also causes an abort.
-#define LOG_TO(dest, severity)                                           \
-  ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::dest, \
-                              ::android::base::severity, -1).stream()
+// Use an if-else statement instead of just an if statement here. So if there is a
+// else statement after LOG() macro, it won't bind to the if statement in the macro.
+// do-while(0) statement doesn't work here. Because we need to support << operator
+// following the macro, like "LOG(DEBUG) << xxx;".
+#define LOG_TO(dest, severity)                                                      \
+  if (LIKELY(::android::base::severity < ::android::base::GetMinimumLogSeverity())) \
+    ;                                                                               \
+  else                                                                              \
+    ::android::base::ErrnoRestorer() ? *(std::ostream*)nullptr :                    \
+      ::android::base::LogMessage(__FILE__, __LINE__,                               \
+          ::android::base::dest,                                                    \
+          ::android::base::severity, -1).stream()
 
 // A variant of LOG that also logs the current errno value. To be used when
 // library calls fail.
-#define PLOG(severity)                                                      \
-  ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::DEFAULT, \
-                              ::android::base::severity, errno).stream()
+#define PLOG(severity) PLOG_TO(DEFAULT, severity)
 
 // Behaves like PLOG, but logs to the specified log ID.
-#define PLOG_TO(dest, severity)                                          \
-  ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::dest, \
-                              ::android::base::severity, errno).stream()
+#define PLOG_TO(dest, severity)                                                     \
+  if (LIKELY(::android::base::severity < ::android::base::GetMinimumLogSeverity())) \
+    ;                                                                               \
+  else                                                                              \
+    ::android::base::ErrnoRestorer() ? *(std::ostream*)nullptr :                    \
+      ::android::base::LogMessage(__FILE__, __LINE__,                               \
+          ::android::base::dest,                                                    \
+          ::android::base::severity, errno).stream()
 
 // Marker that code is yet to be implemented.
 #define UNIMPLEMENTED(level) \
@@ -122,11 +156,13 @@
 //
 //     CHECK(false == true) results in a log message of
 //       "Check failed: false == true".
-#define CHECK(x)                                                            \
-  if (UNLIKELY(!(x)))                                                       \
-  ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::DEFAULT, \
-                              ::android::base::FATAL, -1).stream()          \
-      << "Check failed: " #x << " "
+#define CHECK(x)                                                              \
+  if (LIKELY((x)))                                                            \
+    ;                                                                         \
+  else                                                                        \
+    ::android::base::LogMessage(__FILE__, __LINE__, ::android::base::DEFAULT, \
+                                ::android::base::FATAL, -1).stream()          \
+        << "Check failed: " #x << " "
 
 // Helper for CHECK_xx(x,y) macros.
 #define CHECK_OP(LHS, RHS, OP)                                              \
@@ -153,7 +189,9 @@
 
 // Helper for CHECK_STRxx(s1,s2) macros.
 #define CHECK_STROP(s1, s2, sense)                                         \
-  if (UNLIKELY((strcmp(s1, s2) == 0) != sense))                            \
+  if (LIKELY((strcmp(s1, s2) == 0) == sense))                              \
+    ;                                                                      \
+  else                                                                     \
     LOG(FATAL) << "Check failed: "                                         \
                << "\"" << s1 << "\""                                       \
                << (sense ? " == " : " != ") << "\"" << s2 << "\""
diff --git a/base/logging.cpp b/base/logging.cpp
index e9e06df..6bfaaec 100644
--- a/base/logging.cpp
+++ b/base/logging.cpp
@@ -139,6 +139,10 @@
 static LogSeverity gMinimumLogSeverity = INFO;
 static std::unique_ptr<std::string> gProgramInvocationName;
 
+LogSeverity GetMinimumLogSeverity() {
+  return gMinimumLogSeverity;
+}
+
 static const char* ProgramInvocationName() {
   if (gProgramInvocationName == nullptr) {
     gProgramInvocationName.reset(new std::string(getprogname()));
@@ -200,20 +204,6 @@
   InitLogging(argv);
 }
 
-// TODO: make this public; it's independently useful.
-class ErrnoRestorer {
- public:
-  ErrnoRestorer(int saved_errno) : saved_errno_(saved_errno) {
-  }
-
-  ~ErrnoRestorer() {
-    errno = saved_errno_;
-  }
-
- private:
-  const int saved_errno_;
-};
-
 void InitLogging(char* argv[]) {
   if (gInitialized) {
     return;
@@ -286,13 +276,12 @@
 class LogMessageData {
  public:
   LogMessageData(const char* file, unsigned int line, LogId id,
-                 LogSeverity severity, int error, int saved_errno)
+                 LogSeverity severity, int error)
       : file_(GetFileBasename(file)),
         line_number_(line),
         id_(id),
         severity_(severity),
-        error_(error),
-        errno_restorer_(saved_errno) {
+        error_(error) {
   }
 
   const char* GetFile() const {
@@ -330,39 +319,38 @@
   const LogId id_;
   const LogSeverity severity_;
   const int error_;
-  ErrnoRestorer errno_restorer_;
 
   DISALLOW_COPY_AND_ASSIGN(LogMessageData);
 };
 
 LogMessage::LogMessage(const char* file, unsigned int line, LogId id,
                        LogSeverity severity, int error)
-    : data_(new LogMessageData(file, line, id, severity, error, errno)) {
+    : data_(new LogMessageData(file, line, id, severity, error)) {
 }
 
 LogMessage::~LogMessage() {
-  if (data_->GetSeverity() < gMinimumLogSeverity) {
-    return;  // No need to format something we're not going to output.
-  }
-
   // Finish constructing the message.
   if (data_->GetError() != -1) {
     data_->GetBuffer() << ": " << strerror(data_->GetError());
   }
   std::string msg(data_->ToString());
 
-  if (msg.find('\n') == std::string::npos) {
-    LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetId(),
-            data_->GetSeverity(), msg.c_str());
-  } else {
-    msg += '\n';
-    size_t i = 0;
-    while (i < msg.size()) {
-      size_t nl = msg.find('\n', i);
-      msg[nl] = '\0';
+  {
+    // Do the actual logging with the lock held.
+    lock_guard<mutex> lock(logging_lock);
+    if (msg.find('\n') == std::string::npos) {
       LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetId(),
-              data_->GetSeverity(), &msg[i]);
-      i = nl + 1;
+              data_->GetSeverity(), msg.c_str());
+    } else {
+      msg += '\n';
+      size_t i = 0;
+      while (i < msg.size()) {
+        size_t nl = msg.find('\n', i);
+        msg[nl] = '\0';
+        LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetId(),
+                data_->GetSeverity(), &msg[i]);
+        i = nl + 1;
+      }
     }
   }
 
@@ -382,7 +370,6 @@
 void LogMessage::LogLine(const char* file, unsigned int line, LogId id,
                          LogSeverity severity, const char* message) {
   const char* tag = ProgramInvocationName();
-  lock_guard<mutex> lock(logging_lock);
   gLogger(id, severity, tag, file, line, message);
 }
 
diff --git a/base/logging_test.cpp b/base/logging_test.cpp
index c12dfa5..9cf1aad 100644
--- a/base/logging_test.cpp
+++ b/base/logging_test.cpp
@@ -18,6 +18,10 @@
 
 #include <libgen.h>
 
+#if defined(_WIN32)
+#include <signal.h>
+#endif
+
 #include <regex>
 #include <string>
 
@@ -49,6 +53,11 @@
 
  private:
   void init() {
+#if defined(_WIN32)
+    // On Windows, stderr is often buffered, so make sure it is unbuffered so
+    // that we can immediately read back what was written to stderr.
+    ASSERT_EQ(0, setvbuf(stderr, NULL, _IONBF, 0));
+#endif
     old_stderr_ = dup(STDERR_FILENO);
     ASSERT_NE(-1, old_stderr_);
     ASSERT_NE(-1, dup2(fd(), STDERR_FILENO));
@@ -57,21 +66,58 @@
   void reset() {
     ASSERT_NE(-1, dup2(old_stderr_, STDERR_FILENO));
     ASSERT_EQ(0, close(old_stderr_));
+    // Note: cannot restore prior setvbuf() setting.
   }
 
   TemporaryFile temp_file_;
   int old_stderr_;
 };
 
+#if defined(_WIN32)
+static void ExitSignalAbortHandler(int) {
+  _exit(3);
+}
+#endif
+
+static void SuppressAbortUI() {
+#if defined(_WIN32)
+  // We really just want to call _set_abort_behavior(0, _CALL_REPORTFAULT) to
+  // suppress the Windows Error Reporting dialog box, but that API is not
+  // available in the OS-supplied C Runtime, msvcrt.dll, that we currently
+  // use (it is available in the Visual Studio C runtime).
+  //
+  // Instead, we setup a SIGABRT handler, which is called in abort() right
+  // before calling Windows Error Reporting. In the handler, we exit the
+  // process just like abort() does.
+  ASSERT_NE(SIG_ERR, signal(SIGABRT, ExitSignalAbortHandler));
+#endif
+}
+
 TEST(logging, CHECK) {
-  ASSERT_DEATH(CHECK(false), "Check failed: false ");
+  ASSERT_DEATH({SuppressAbortUI(); CHECK(false);}, "Check failed: false ");
   CHECK(true);
 
-  ASSERT_DEATH(CHECK_EQ(0, 1), "Check failed: 0 == 1 ");
+  ASSERT_DEATH({SuppressAbortUI(); CHECK_EQ(0, 1);}, "Check failed: 0 == 1 ");
   CHECK_EQ(0, 0);
 
-  ASSERT_DEATH(CHECK_STREQ("foo", "bar"), R"(Check failed: "foo" == "bar")");
+  ASSERT_DEATH({SuppressAbortUI(); CHECK_STREQ("foo", "bar");},
+               R"(Check failed: "foo" == "bar")");
   CHECK_STREQ("foo", "foo");
+
+  // Test whether CHECK() and CHECK_STREQ() have a dangling if with no else.
+  bool flag = false;
+  if (true)
+    CHECK(true);
+  else
+    flag = true;
+  EXPECT_FALSE(flag) << "CHECK macro probably has a dangling if with no else";
+
+  flag = false;
+  if (true)
+    CHECK_STREQ("foo", "foo");
+  else
+    flag = true;
+  EXPECT_FALSE(flag) << "CHECK_STREQ probably has a dangling if with no else";
 }
 
 std::string make_log_pattern(android::base::LogSeverity severity,
@@ -85,7 +131,7 @@
 }
 
 TEST(logging, LOG) {
-  ASSERT_DEATH(LOG(FATAL) << "foobar", "foobar");
+  ASSERT_DEATH({SuppressAbortUI(); LOG(FATAL) << "foobar";}, "foobar");
 
   // We can't usefully check the output of any of these on Windows because we
   // don't have std::regex, but we can at least make sure we printed at least as
@@ -148,6 +194,50 @@
     ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
 #endif
   }
+
+  // Test whether LOG() saves and restores errno.
+  {
+    CapturedStderr cap;
+    errno = 12345;
+    LOG(INFO) << (errno = 67890);
+    EXPECT_EQ(12345, errno) << "errno was not restored";
+
+    ASSERT_EQ(0, lseek(cap.fd(), SEEK_SET, 0));
+
+    std::string output;
+    android::base::ReadFdToString(cap.fd(), &output);
+    EXPECT_NE(nullptr, strstr(output.c_str(), "67890")) << output;
+
+#if !defined(_WIN32)
+    std::regex message_regex(
+        make_log_pattern(android::base::INFO, "67890"));
+    ASSERT_TRUE(std::regex_search(output, message_regex)) << output;
+#endif
+  }
+
+  // Test whether LOG() has a dangling if with no else.
+  {
+    CapturedStderr cap;
+
+    // Do the test two ways: once where we hypothesize that LOG()'s if
+    // will evaluate to true (when severity is high enough) and once when we
+    // expect it to evaluate to false (when severity is not high enough).
+    bool flag = false;
+    if (true)
+      LOG(INFO) << "foobar";
+    else
+      flag = true;
+
+    EXPECT_FALSE(flag) << "LOG macro probably has a dangling if with no else";
+
+    flag = false;
+    if (true)
+      LOG(VERBOSE) << "foobar";
+    else
+      flag = true;
+
+    EXPECT_FALSE(flag) << "LOG macro probably has a dangling if with no else";
+  }
 }
 
 TEST(logging, PLOG) {
diff --git a/logcat/Android.mk b/logcat/Android.mk
index 844ab8b..c4a9550 100644
--- a/logcat/Android.mk
+++ b/logcat/Android.mk
@@ -15,4 +15,15 @@
 
 include $(BUILD_EXECUTABLE)
 
+include $(CLEAR_VARS)
+
+LOCAL_MODULE := logpersist.start
+LOCAL_MODULE_TAGS := debug
+LOCAL_MODULE_CLASS := EXECUTABLES
+LOCAL_MODULE_PATH := $(bin_dir)
+LOCAL_SRC_FILES := logpersist
+ALL_TOOLS := logpersist.start logpersist.stop logpersist.cat
+LOCAL_POST_INSTALL_CMD := $(hide) $(foreach t,$(filter-out $(LOCAL_MODULE),$(ALL_TOOLS)),ln -sf $(LOCAL_MODULE) $(TARGET_OUT)/bin/$(t);)
+include $(BUILD_PREBUILT)
+
 include $(call first-makefiles-under,$(LOCAL_PATH))
diff --git a/logcat/logcatd.rc b/logcat/logcatd.rc
index 0bc581e..33d39ac 100644
--- a/logcat/logcatd.rc
+++ b/logcat/logcatd.rc
@@ -2,10 +2,10 @@
     # all exec/services are called with umask(077), so no gain beyond 0700
     mkdir /data/misc/logd 0700 logd log
     # logd for write to /data/misc/logd, log group for read from pstore (-L)
-    exec - logd log -- /system/bin/logcat -L -b all -v threadtime -v usec -v printable -D -f /data/misc/logd/logcat -r 64 -n 256
+    exec - logd log -- /system/bin/logcat -L -b all -v threadtime -v usec -v printable -D -f /data/misc/logd/logcat -r 1024 -n 256
     start logcatd
 
-service logcatd /system/bin/logcat -b all -v threadtime -v usec -v printable -D -f /data/misc/logd/logcat -r 64 -n 256
+service logcatd /system/bin/logcat -b all -v threadtime -v usec -v printable -D -f /data/misc/logd/logcat -r 1024 -n 256
     class late_start
     disabled
     # logd for write to /data/misc/logd, log group for read from log daemon
diff --git a/logd/logpersist b/logcat/logpersist
similarity index 88%
rename from logd/logpersist
rename to logcat/logpersist
index 215e1e2..6f666f6 100755
--- a/logd/logpersist
+++ b/logcat/logpersist
@@ -1,9 +1,15 @@
 #! /system/bin/sh
 # logpersist cat start and stop handlers
+progname="${0##*/}"
+case `getprop ro.build.type` in
+userdebug|eng) ;;
+*) echo "${progname} - Permission denied"
+   exit 1
+   ;;
+esac
 data=/data/misc/logd
 property=persist.logd.logpersistd
 service=logcatd
-progname="${0##*/}"
 if [ X"${1}" = "-h" -o X"${1}" = X"--help" ]; then
   echo "${progname%.*}.cat            - dump current ${service%d} logs"
   echo "${progname%.*}.start          - start ${service} service"
diff --git a/logd/Android.mk b/logd/Android.mk
index 01c51c7..c00061b 100644
--- a/logd/Android.mk
+++ b/logd/Android.mk
@@ -43,15 +43,4 @@
 
 include $(BUILD_EXECUTABLE)
 
-include $(CLEAR_VARS)
-
-LOCAL_MODULE := logpersist.start
-LOCAL_MODULE_TAGS := debug
-LOCAL_MODULE_CLASS := EXECUTABLES
-LOCAL_MODULE_PATH := $(bin_dir)
-LOCAL_SRC_FILES := logpersist
-ALL_TOOLS := logpersist.start logpersist.stop logpersist.cat
-LOCAL_POST_INSTALL_CMD := $(hide) $(foreach t,$(filter-out $(LOCAL_MODULE),$(ALL_TOOLS)),ln -sf $(LOCAL_MODULE) $(TARGET_OUT)/bin/$(t);)
-include $(BUILD_PREBUILT)
-
 include $(call first-makefiles-under,$(LOCAL_PATH))
diff --git a/metricsd/metrics_daemon_test.cc b/metricsd/metrics_daemon_test.cc
index 476d0f3..1adf9de 100644
--- a/metricsd/metrics_daemon_test.cc
+++ b/metricsd/metrics_daemon_test.cc
@@ -25,6 +25,7 @@
 #include <base/files/scoped_temp_dir.h>
 #include <base/strings/string_number_conversions.h>
 #include <base/strings/stringprintf.h>
+#include <chromeos/flag_helper.h>
 #include <gtest/gtest.h>
 
 #include "constants.h"
@@ -60,6 +61,7 @@
   std::string kFakeDiskStats1;
 
   virtual void SetUp() {
+    chromeos::FlagHelper::Init(0, nullptr, "");
     EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
     scaling_max_freq_path_ = temp_dir_.path().Append("scaling_max");
     cpu_max_freq_path_ = temp_dir_.path().Append("cpu_freq_max");