Merge "Make NativePattern.address field private"
diff --git a/android_icu4j/src/main/java/com/android/icu/util/regex/NativeMatcher.java b/android_icu4j/src/main/java/com/android/icu/util/regex/NativeMatcher.java
index 9165b3e..20e562b 100644
--- a/android_icu4j/src/main/java/com/android/icu/util/regex/NativeMatcher.java
+++ b/android_icu4j/src/main/java/com/android/icu/util/regex/NativeMatcher.java
@@ -23,11 +23,6 @@
     private static final NativeAllocationRegistry REGISTRY = NativeAllocationRegistry
         .createMalloced(NativeMatcher.class.getClassLoader(), getNativeFinalizer());
 
-    /*
-     * @ReachabilitySensitive ensures that this and NativePattern remain reachable while we're using
-     * nativePattern.address directly.
-     */
-    @dalvik.annotation.optimization.ReachabilitySensitive
     private final NativePattern nativePattern;
     @dalvik.annotation.optimization.ReachabilitySensitive
     private final long address;
@@ -39,13 +34,13 @@
 
     private NativeMatcher(NativePattern pattern) {
         nativePattern = pattern;
-        address = openImpl(pattern.address);
+        address = pattern.openMatcher();
         REGISTRY.registerNativeAllocation(this, address);
     }
 
     @libcore.api.IntraCoreApi
     public int getMatchedGroupIndex(String groupName) {
-        return getMatchedGroupIndexImpl(nativePattern.address, groupName);
+        return nativePattern.getMatchedGroupIndex(groupName);
     }
 
     @libcore.api.IntraCoreApi
@@ -98,8 +93,6 @@
         useTransparentBoundsImpl(address, value);
     }
 
-    private static native long openImpl(long patternAddr);
-    private static native int getMatchedGroupIndexImpl(long patternAddr, String name);
     private static native boolean findImpl(long addr, int startIndex, int[] offsets);
     private static native boolean findNextImpl(long addr, int[] offsets);
     private static native int groupCountImpl(long addr);
diff --git a/android_icu4j/src/main/java/com/android/icu/util/regex/NativePattern.java b/android_icu4j/src/main/java/com/android/icu/util/regex/NativePattern.java
index 2d0a6ca..8771918 100644
--- a/android_icu4j/src/main/java/com/android/icu/util/regex/NativePattern.java
+++ b/android_icu4j/src/main/java/com/android/icu/util/regex/NativePattern.java
@@ -28,7 +28,7 @@
         .createMalloced(NativePattern.class.getClassLoader(), getNativeFinalizer());
 
     @dalvik.annotation.optimization.ReachabilitySensitive
-    final long address;
+    private final long address;
 
     @libcore.api.IntraCoreApi
     public static NativePattern create(String pattern, int flags) {
@@ -40,6 +40,14 @@
         REGISTRY.registerNativeAllocation(this, address);
     }
 
+    /* package */ int getMatchedGroupIndex(String groupName) {
+        return getMatchedGroupIndexImpl(address, groupName);
+    }
+
+    /* package */ long openMatcher() {
+        return openMatcherImpl(address);
+    }
+
     /**
      * @return native address of the native allocation.
      */
@@ -51,4 +59,16 @@
      */
     private static native long getNativeFinalizer();
 
+    /**
+     * @param addr the NativePattern.address
+     * @return native address of matcher implementation
+     */
+    private static native long openMatcherImpl(long addr);
+
+    /**
+     * @param groupName The name of a named-capturing group
+     * @return the index of the named-capturing group
+     */
+    private static native int getMatchedGroupIndexImpl(long addr, String groupName);
+
 }
diff --git a/android_icu4j/src/main/jni/JniConstants.cpp b/android_icu4j/src/main/jni/JniConstants.cpp
index 88126e8..56483b6 100644
--- a/android_icu4j/src/main/jni/JniConstants.cpp
+++ b/android_icu4j/src/main/jni/JniConstants.cpp
@@ -73,6 +73,10 @@
 }
 
 void JniConstants::Invalidate() {
+    // Clean shutdown would require calling DeleteGlobalRef() for each of the
+    // class references. However, JavaVM can't be used for cleanup during
+    // JNI_OnUnload because ART only calls this once all threads are
+    // unregistered.
     std::lock_guard guard(g_constants_mutex);
     g_constants_valid = false;
 }
\ No newline at end of file
diff --git a/android_icu4j/src/main/jni/MatcherState.cpp b/android_icu4j/src/main/jni/MatcherState.cpp
new file mode 100644
index 0000000..0a8e130
--- /dev/null
+++ b/android_icu4j/src/main/jni/MatcherState.cpp
@@ -0,0 +1,108 @@
+/*
+ * Copyright (C) 2010 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.
+ */
+
+#define LOG_TAG "MatcherState"
+
+#include "IcuUtilities.h"
+#include "MatcherState.h"
+
+#include <android-base/logging.h>
+
+#include <nativehelper/ScopedPrimitiveArray.h>
+#include <nativehelper/ScopedStringChars.h>
+
+
+MatcherState::MatcherState(icu::RegexMatcher* matcher) :
+    mMatcher(matcher),
+    mUChars(nullptr),
+    mUText(nullptr),
+    mStatus(U_ZERO_ERROR) {
+}
+
+bool MatcherState::updateInput(JNIEnv* env, jstring input) {
+    // First, close the UText struct, since we're about to allocate a new one.
+    if (mUText != nullptr) {
+        utext_close(mUText);
+        mUText = nullptr;
+    }
+
+    // Then delete the UChar* associated with the UText struct..
+    mUChars.reset(nullptr);
+
+    // TODO: We should investigate whether we can avoid an additional copy
+    // in the native heap when is_copy == JNI_TRUE. The problem with doing
+    // that is that we might call ReleaseStringChars with a different
+    // JNIEnv* on a different downcall. This is currently safe as
+    // implemented in ART, but is unlikely to be portable and the spec stays
+    // silent on the matter.
+    ScopedStringChars inputChars(env, input);
+    if (inputChars.get() == nullptr) {
+        // There will be an exception pending if we get here.
+        return false;
+    }
+
+    // Make a copy of |input| on the native heap. This copy will be live
+    // until the next call to updateInput or close.
+    mUChars.reset(new (std::nothrow) UChar[inputChars.size()]);
+    if (mUChars.get() == nullptr) {
+        env->ThrowNew(env->FindClass("Ljava/lang/OutOfMemoryError;"), "Out of memory");
+        return false;
+    }
+
+    static_assert(sizeof(UChar) == sizeof(jchar), "sizeof(Uchar) != sizeof(jchar)");
+    memcpy(mUChars.get(), inputChars.get(), inputChars.size() * sizeof(jchar));
+
+    // Reset any errors that might have occurred on previous patches.
+    mStatus = U_ZERO_ERROR;
+    mUText = utext_openUChars(nullptr, mUChars.get(), inputChars.size(), &mStatus);
+    if (mUText == nullptr) {
+        CHECK(maybeThrowIcuException(env, "utext_openUChars", mStatus));
+        return false;
+    }
+
+    // It is an error for ICU to have returned a non-null mUText but to
+    // still have indicated an error.
+    CHECK(U_SUCCESS(mStatus));
+
+    mMatcher->reset(mUText);
+    return true;
+}
+
+MatcherState::~MatcherState() {
+    if (mUText != nullptr) {
+        utext_close(mUText);
+    }
+}
+
+icu::RegexMatcher* MatcherState::matcher() {
+    return mMatcher.get();
+}
+
+UErrorCode& MatcherState::status() {
+    return mStatus;
+}
+
+void MatcherState::updateOffsets(JNIEnv* env, jintArray javaOffsets) {
+    ScopedIntArrayRW offsets(env, javaOffsets);
+    if (offsets.get() == NULL) {
+        return;
+    }
+
+    for (size_t i = 0, groupCount = mMatcher->groupCount(); i <= groupCount; ++i) {
+        offsets[2*i + 0] = mMatcher->start(i, mStatus);
+        offsets[2*i + 1] = mMatcher->end(i, mStatus);
+    }
+}
diff --git a/android_icu4j/src/main/jni/MatcherState.h b/android_icu4j/src/main/jni/MatcherState.h
new file mode 100644
index 0000000..9a5f3c5
--- /dev/null
+++ b/android_icu4j/src/main/jni/MatcherState.h
@@ -0,0 +1,61 @@
+/*
+ * Copyright (C) 2010 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.
+ */
+
+#pragma once
+
+#include <nativehelper/JNIHelp.h>
+#include <nativehelper/jni_macros.h>
+
+#include "unicode/parseerr.h"
+#include "unicode/regex.h"
+
+/**
+ * Encapsulates an instance of ICU4C's RegexMatcher class along with a copy of
+ * the input it's currently operating on in the native heap.
+ *
+ * Rationale: We choose to make a copy here because it turns out to be a lot
+ * cheaper when a moving GC and/or string compression is enabled. This is
+ * because env->GetStringChars() always copies in this scenario. This becomes
+ * especially bad when the String in question is long and/or contains a large
+ * number of matches.
+ *
+ * Drawbacks: The native allocation associated with this class is no longer
+ * fixed size, but NativeAllocationRegistry should be able to determine
+ * the native heap size by mallinfo().
+ */
+class MatcherState {
+public:
+    MatcherState(icu::RegexMatcher* matcher);
+    ~MatcherState();
+
+    bool updateInput(JNIEnv* env, jstring input);
+
+    icu::RegexMatcher* matcher();
+
+    UErrorCode& status();
+
+    void updateOffsets(JNIEnv* env, jintArray javaOffsets);
+
+private:
+    std::unique_ptr<icu::RegexMatcher> mMatcher;
+    std::unique_ptr<UChar[]> mUChars;
+    UText* mUText;
+    UErrorCode mStatus;
+
+    // Disallow copy and assignment.
+    MatcherState(const MatcherState&);
+    void operator=(const MatcherState&);
+};
\ No newline at end of file
diff --git a/android_icu4j/src/main/jni/com_android_icu_util_regex_NativeMatcher.cpp b/android_icu4j/src/main/jni/com_android_icu_util_regex_NativeMatcher.cpp
index 8e5e200..5a3882b 100644
--- a/android_icu4j/src/main/jni/com_android_icu_util_regex_NativeMatcher.cpp
+++ b/android_icu4j/src/main/jni/com_android_icu_util_regex_NativeMatcher.cpp
@@ -26,123 +26,13 @@
 
 #include <android-base/logging.h>
 #include "IcuUtilities.h"
+#include "MatcherState.h"
 #include "ScopedJavaUnicodeString.h"
 #include "unicode/parseerr.h"
 #include "unicode/regex.h"
 
 // ICU documentation: http://icu-project.org/apiref/icu4c/classRegexMatcher.html
 
-/**
- * Encapsulates an instance of ICU4C's RegexMatcher class along with a copy of
- * the input it's currently operating on in the native heap.
- *
- * Rationale: We choose to make a copy here because it turns out to be a lot
- * cheaper when a moving GC and/or string compression is enabled. This is
- * because env->GetStringChars() always copies in this scenario. This becomes
- * especially bad when the String in question is long and/or contains a large
- * number of matches.
- *
- * Drawbacks: The native allocation associated with this class is no longer
- * fixed size, so we're effectively lying to the NativeAllocationRegistry about
- * the size of the object(s) we're allocating on the native heap. The peak
- * memory usage doesn't change though, given that GetStringChars would have
- * made an allocation of precisely the same size.
- */
-class MatcherState {
-public:
-    MatcherState(icu::RegexMatcher* matcher) :
-        mMatcher(matcher),
-        mUChars(nullptr),
-        mUText(nullptr),
-        mStatus(U_ZERO_ERROR) {
-    }
-
-    bool updateInput(JNIEnv* env, jstring input) {
-        // First, close the UText struct, since we're about to allocate a new one.
-        if (mUText != nullptr) {
-            utext_close(mUText);
-            mUText = nullptr;
-        }
-
-        // Then delete the UChar* associated with the UText struct..
-        mUChars.reset(nullptr);
-
-        // TODO: We should investigate whether we can avoid an additional copy
-        // in the native heap when is_copy == JNI_TRUE. The problem with doing
-        // that is that we might call ReleaseStringChars with a different
-        // JNIEnv* on a different downcall. This is currently safe as
-        // implemented in ART, but is unlikely to be portable and the spec stays
-        // silent on the matter.
-        ScopedStringChars inputChars(env, input);
-        if (inputChars.get() == nullptr) {
-            // There will be an exception pending if we get here.
-            return false;
-        }
-
-        // Make a copy of |input| on the native heap. This copy will be live
-        // until the next call to updateInput or close.
-        mUChars.reset(new (std::nothrow) UChar[inputChars.size()]);
-        if (mUChars.get() == nullptr) {
-            env->ThrowNew(env->FindClass("Ljava/lang/OutOfMemoryError;"), "Out of memory");
-            return false;
-        }
-
-        static_assert(sizeof(UChar) == sizeof(jchar), "sizeof(Uchar) != sizeof(jchar)");
-        memcpy(mUChars.get(), inputChars.get(), inputChars.size() * sizeof(jchar));
-
-        // Reset any errors that might have occurred on previous patches.
-        mStatus = U_ZERO_ERROR;
-        mUText = utext_openUChars(nullptr, mUChars.get(), inputChars.size(), &mStatus);
-        if (mUText == nullptr) {
-            CHECK(maybeThrowIcuException(env, "utext_openUChars", mStatus));
-            return false;
-        }
-
-        // It is an error for ICU to have returned a non-null mUText but to
-        // still have indicated an error.
-        CHECK(U_SUCCESS(mStatus));
-
-        mMatcher->reset(mUText);
-        return true;
-    }
-
-    ~MatcherState() {
-        if (mUText != nullptr) {
-            utext_close(mUText);
-        }
-    }
-
-    icu::RegexMatcher* matcher() {
-        return mMatcher.get();
-    }
-
-    UErrorCode& status() {
-        return mStatus;
-    }
-
-    void updateOffsets(JNIEnv* env, jintArray javaOffsets) {
-        ScopedIntArrayRW offsets(env, javaOffsets);
-        if (offsets.get() == NULL) {
-            return;
-        }
-
-        for (size_t i = 0, groupCount = mMatcher->groupCount(); i <= groupCount; ++i) {
-            offsets[2*i + 0] = mMatcher->start(i, mStatus);
-            offsets[2*i + 1] = mMatcher->end(i, mStatus);
-        }
-    }
-
-private:
-    std::unique_ptr<icu::RegexMatcher> mMatcher;
-    std::unique_ptr<UChar[]> mUChars;
-    UText* mUText;
-    UErrorCode mStatus;
-
-    // Disallow copy and assignment.
-    MatcherState(const MatcherState&);
-    void operator=(const MatcherState&);
-};
-
 static inline MatcherState* toMatcherState(jlong address) {
     return reinterpret_cast<MatcherState*>(static_cast<uintptr_t>(address));
 }
@@ -214,17 +104,6 @@
     }
 }
 
-static jlong NativeMatcher_openImpl(JNIEnv* env, jclass, jlong patternAddr) {
-    icu::RegexPattern* pattern = reinterpret_cast<icu::RegexPattern*>(static_cast<uintptr_t>(patternAddr));
-    UErrorCode status = U_ZERO_ERROR;
-    icu::RegexMatcher* result = pattern->matcher(status);
-    if (maybeThrowIcuException(env, "RegexPattern::matcher", status)) {
-        return 0;
-    }
-
-    return reinterpret_cast<uintptr_t>(new MatcherState(result));
-}
-
 static jboolean NativeMatcher_requireEndImpl(JNIEnv*, jclass, jlong addr) {
     MatcherState* state = toMatcherState(addr);
     if (state->matcher()->requireEnd() != 0) {
@@ -251,25 +130,8 @@
     state->matcher()->useTransparentBounds(value);
 }
 
-static jint NativeMatcher_getMatchedGroupIndexImpl(JNIEnv* env, jclass, jlong patternAddr, jstring javaGroupName) {
-  icu::RegexPattern* pattern = reinterpret_cast<icu::RegexPattern*>(static_cast<uintptr_t>(patternAddr));
-  ScopedJavaUnicodeString groupName(env, javaGroupName);
-  UErrorCode status = U_ZERO_ERROR;
-
-  jint result = pattern->groupNumberFromName(groupName.unicodeString(), status);
-  if (U_SUCCESS(status)) {
-    return result;
-  }
-  if (status == U_REGEX_INVALID_CAPTURE_GROUP_NAME) {
-    return -1;
-  }
-  maybeThrowIcuException(env, "RegexPattern::groupNumberFromName", status);
-  return -1;
-}
-
 
 static JNINativeMethod gMethods[] = {
-    NATIVE_METHOD(NativeMatcher, getMatchedGroupIndexImpl, "(JLjava/lang/String;)I"),
     NATIVE_METHOD(NativeMatcher, findImpl, "(JI[I)Z"),
     NATIVE_METHOD(NativeMatcher, findNextImpl, "(J[I)Z"),
     NATIVE_METHOD(NativeMatcher, getNativeFinalizer, "()J"),
@@ -277,7 +139,6 @@
     NATIVE_METHOD(NativeMatcher, hitEndImpl, "(J)Z"),
     NATIVE_METHOD(NativeMatcher, lookingAtImpl, "(J[I)Z"),
     NATIVE_METHOD(NativeMatcher, matchesImpl, "(J[I)Z"),
-    NATIVE_METHOD(NativeMatcher, openImpl, "(J)J"),
     NATIVE_METHOD(NativeMatcher, requireEndImpl, "(J)Z"),
     NATIVE_METHOD(NativeMatcher, setInputImpl, "(JLjava/lang/String;II)V"),
     NATIVE_METHOD(NativeMatcher, useAnchoringBoundsImpl, "(JZ)V"),
diff --git a/android_icu4j/src/main/jni/com_android_icu_util_regex_NativePattern.cpp b/android_icu4j/src/main/jni/com_android_icu_util_regex_NativePattern.cpp
index f93049c..5833d75 100644
--- a/android_icu4j/src/main/jni/com_android_icu_util_regex_NativePattern.cpp
+++ b/android_icu4j/src/main/jni/com_android_icu_util_regex_NativePattern.cpp
@@ -21,6 +21,8 @@
 #include <nativehelper/JNIHelp.h>
 #include <nativehelper/jni_macros.h>
 
+#include "IcuUtilities.h"
+#include "MatcherState.h"
 #include "unicode/parseerr.h"
 #include "unicode/regex.h"
 
@@ -96,10 +98,39 @@
     return static_cast<jlong>(reinterpret_cast<uintptr_t>(result));
 }
 
+static jlong NativePattern_openMatcherImpl(JNIEnv* env, jclass, jlong addr) {
+    icu::RegexPattern* pattern = reinterpret_cast<icu::RegexPattern*>(static_cast<uintptr_t>(addr));
+    UErrorCode status = U_ZERO_ERROR;
+    icu::RegexMatcher* result = pattern->matcher(status);
+    if (maybeThrowIcuException(env, "RegexPattern::matcher", status)) {
+        return 0;
+    }
+
+    return reinterpret_cast<uintptr_t>(new MatcherState(result));
+}
+
+static jint NativePattern_getMatchedGroupIndexImpl(JNIEnv* env, jclass, jlong addr, jstring javaGroupName) {
+  icu::RegexPattern* pattern = reinterpret_cast<icu::RegexPattern*>(static_cast<uintptr_t>(addr));
+  ScopedJavaUnicodeString groupName(env, javaGroupName);
+  UErrorCode status = U_ZERO_ERROR;
+
+  jint result = pattern->groupNumberFromName(groupName.unicodeString(), status);
+  if (U_SUCCESS(status)) {
+    return result;
+  }
+  if (status == U_REGEX_INVALID_CAPTURE_GROUP_NAME) {
+    return -1;
+  }
+  maybeThrowIcuException(env, "RegexPattern::groupNumberFromName", status);
+  return -1;
+}
+
 
 static JNINativeMethod gMethods[] = {
     NATIVE_METHOD(NativePattern, compileImpl, "(Ljava/lang/String;I)J"),
     NATIVE_METHOD(NativePattern, getNativeFinalizer, "()J"),
+    NATIVE_METHOD(NativePattern, openMatcherImpl, "(J)J"),
+    NATIVE_METHOD(NativePattern, getMatchedGroupIndexImpl, "(JLjava/lang/String;)I"),
 };
 
 void register_com_android_icu_util_regex_NativePattern(JNIEnv* env) {