[Cronet] Enable persistence mode for Sdch

components/cronet/android/test/sdch_test_util.cc

Index: components/cronet/android/test/sdch_test_util.cc
diff --git a/components/cronet/android/test/sdch_test_util.cc b/components/cronet/android/test/sdch_test_util.cc
new file mode 100644
index 0000000000000000000000000000000000000000..5860e74340d77d190b776348c37a4806e1771728
--- /dev/null
+++ b/components/cronet/android/test/sdch_test_util.cc
@@ -0,0 +1,112 @@
+// Copyright 2015 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.
+
+#include "sdch_test_util.h"
+
+#include <string>
+
+#include "base/android/jni_android.h"
+#include "base/android/jni_string.h"
+#include "base/bind.h"
+#include "base/macros.h"
+#include "components/cronet/android/cronet_url_request_context_adapter.h"
+#include "components/cronet/android/url_request_context_adapter.h"
+#include "jni/SdchTestUtil_jni.h"
+#include "net/base/sdch_manager.h"
+#include "net/base/sdch_observer.h"
+#include "net/url_request/url_request_context.h"
+#include "url/gurl.h"
+
+namespace cronet {
+
+namespace {
+
+class TestSdchObserver : public net::SdchObserver {
+ public:
+  TestSdchObserver(net::SdchManager* manager) : manager_(manager) {}
+
+  // SdchObserver implementation
+  void OnDictionaryAdded(const GURL& dictionary_url,
+                         const std::string& server_hash) override {
+    JNIEnv* env = base::android::AttachCurrentThread();
+    Java_SdchTestUtil_onDictionaryAdded(
+        env, base::android::ConvertUTF8ToJavaString(env, dictionary_url.spec())
+                 .Release());

[mmenke, 2015/05/22 15:46:41]

I think this is a leak, and you should actually be using obj().

Release doesn't release a reference, so it's what you should use when returning
things back to Java, but when passing them in to Java, Java adds a reference to
them itself.  At least other JNI calls into Java seem to use obj().

[xunjieli, 2015/05/22 16:08:31]

Done. Ah, yes, you are right!

+    manager_->RemoveObserver(this);
+    delete this;
+  }
+
+  void OnDictionaryRemoved(const std::string& server_hash) override{};

[mmenke, 2015/05/22 15:46:41]

nit:  Add space:  "override {}" (x4)

[xunjieli, 2015/05/22 16:08:31]

Hmm.. "git cl format" will revert the blank space. I don't see this mentioned in
the style guide. Should I file a bug against clang format?

[mmenke, 2015/05/26 16:36:21]

Remove the unnecessary semi-colons after the close brace.  Suspect that's
confusing clang format.

+
+  void OnDictionaryUsed(const std::string& server_hash) override{};
+
+  void OnGetDictionary(const GURL& request_url,
+                       const GURL& dictionary_url) override{};
+
+  void OnClearDictionaries() override{};
+
+ private:
+  net::SdchManager* manager_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSdchObserver);
+};
+
+void AddSdchObserverHelper(const GURL& target_url,
+                           net::URLRequestContext* url_request_context) {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  // If dictionaries are already added, skip adding the observer.
+  if (url_request_context->sdch_manager()->GetDictionarySet(target_url)) {
+    Java_SdchTestUtil_onAddSdchObserverCompleted(env, JNI_FALSE);

[mmenke, 2015/05/22 15:46:41]

Calling this a failure seems odd.  Maybe
Java_SdchTestUtil_dictionaryAlreadyPresent?

Also seems like the tets would be clearer if we added a method or an argument to
SdchObserverCallback indicating what path we followed instead (Already present
vs added).  Think it would result in clearer logic for tests.

[xunjieli, 2015/05/22 16:08:31]

Done.

+    return;
+  }
+
+  url_request_context->sdch_manager()->AddObserver(
+      new TestSdchObserver(url_request_context->sdch_manager()));
+  Java_SdchTestUtil_onAddSdchObserverCompleted(env, JNI_TRUE);
+}
+
+void AddSdchObserverOnNetworkThread(
+    const GURL& target_url,
+    CronetURLRequestContextAdapter* context_adapter) {
+  AddSdchObserverHelper(target_url, context_adapter->GetURLRequestContext());
+}
+
+// TODO(xunjieli): Delete this once legacy API is removed.
+void AddSdchObserverOnNetworkThreadLegacyAPI(
+    const GURL& target_url,
+    URLRequestContextAdapter* context_adapter) {
+  AddSdchObserverHelper(target_url, context_adapter->GetURLRequestContext());
+}
+
+}  // namespace
+
+void AddSdchObserver(JNIEnv* env,
+                     jclass jcaller,
+                     jstring jtarget_url,
+                     jlong jadapter) {
+  GURL target_url(base::android::ConvertJavaStringToUTF8(env, jtarget_url));
+  CronetURLRequestContextAdapter* context_adapter =
+      reinterpret_cast<CronetURLRequestContextAdapter*>(jadapter);
+  context_adapter->PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&AddSdchObserverOnNetworkThread, target_url,
+                            base::Unretained(context_adapter)));
+}
+
+void AddSdchObserverLegacyAPI(JNIEnv* env,
+                              jclass jcaller,
+                              jstring jtarget_url,
+                              jlong jadapter) {
+  GURL target_url(base::android::ConvertJavaStringToUTF8(env, jtarget_url));
+  URLRequestContextAdapter* context_adapter =
+      reinterpret_cast<URLRequestContextAdapter*>(jadapter);
+  context_adapter->PostTaskToNetworkThread(
+      FROM_HERE, base::Bind(&AddSdchObserverOnNetworkThreadLegacyAPI,
+                            target_url, base::Unretained(context_adapter)));
+}
+
+bool RegisterSdchTestUtil(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
+
+}  // namespace cronet