Revert "[aw] Allow to set Network in AwPacProcessor."

This reverts commit bd3b0ee6.

Reason for revert: Likely the cause of of the compile crash on
https://ci.chromium.org/p/chrome/builders/ci/arm64-unpublished-builder-rel/14332

Error:
../../clank/android_webview/next/java/src/com/android/webview/chromium/PacProcessorForS.java:34: error: constructor AwPacProcessor in class AwPacProcessor cannot be applied to given types;
        mProcessor = new AwPacProcessor(network);
                     ^
  required: no arguments
  found: Network
  reason: actual and formal argument lists differ in length
1 error

Original change's description:
> [aw] Allow to set Network in AwPacProcessor.
>
> Bug: 1085115
> Change-Id: I8846261a0f21ecb48692b70ab778f68996843760
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414173
> Reviewed-by: Richard Coles <torne@chromium.org>
> Commit-Queue: Anna Malova <amalova@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#812494}

TBR=torne@chromium.org,amalova@chromium.org

Change-Id: I087cdfa4fd0f4ca71c50a38b3a01fec1f67fa7aa
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1085115
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2440600Reviewed-by: Alice Wang <aliceywang@chromium.org>
Commit-Queue: Alice Wang <aliceywang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812582}

android_webview/browser/aw_pac_processor.cc

@@ -108,6 +108,9 @@ proxy_resolver::ProxyResolverV8TracingFactory* GetProxyResolverFactory() {
 // blocking DNS queries, to get better performance.
 class HostResolver : public proxy_resolver::ProxyHostResolver {
  public:
+  HostResolver(net_handle_t net_handle) : net_handle_(net_handle) {}
+  ~HostResolver() override {}
+
   std::unique_ptr<proxy_resolver::ProxyHostResolver::Request> CreateRequest(
       const std::string& hostname,
       net::ProxyResolveDnsOperation operation,
@@ -116,11 +119,9 @@ class HostResolver : public proxy_resolver::ProxyHostResolver {
                                          link_addresses_);
   }
 
-  void SetNetworkAndLinkAddresses(
-      const net_handle_t net_handle,
+  void SetNetworkLinkAddresses(
       const std::vector<net::IPAddress>& link_addresses) {
     link_addresses_ = link_addresses;
-    net_handle_ = net_handle;
   }
 
  private:
@@ -168,7 +169,7 @@ class HostResolver : public proxy_resolver::ProxyHostResolver {
     bool MyIpAddressImpl() {
       // For network-aware queries the results are set from Java on
       // NetworkCallback#onLinkPropertiesChanged.
-      // See SetNetworkAndLinkAddresses.
+      // See SetNetworkLinkAddresses.
       if (IsNetworkSpecified()) {
         results_.push_back(link_addresses_.front());
         return true;
@@ -379,8 +380,9 @@ class MakeProxyRequestJob : public Job {
   std::unique_ptr<net::ProxyResolver::Request> request_;
 };
 
-AwPacProcessor::AwPacProcessor() {
-  host_resolver_ = std::make_unique<HostResolver>();
+AwPacProcessor::AwPacProcessor(net_handle_t net_handle)
+    : net_handle_(net_handle) {
+  host_resolver_ = std::make_unique<HostResolver>(net_handle_);
 }
 
 AwPacProcessor::~AwPacProcessor() {
@@ -465,26 +467,30 @@ ScopedJavaLocalRef<jstring> AwPacProcessor::MakeProxyRequest(
   return ConvertUTF8ToJavaString(env, MakeProxyRequest(url));
 }
 
-void AwPacProcessor::SetNetworkAndLinkAddresses(
+// ProxyResolverV8Tracing posts DNS resolution queries back to the thread
+// it is called from. Post update of link addresses to the same thread to
+// prevent concurrent access and modification of the same vector.
+void AwPacProcessor::SetNetworkLinkAddresses(
     JNIEnv* env,
-    net_handle_t net_handle,
     const base::android::JavaParamRef<jobjectArray>& jlink_addresses) {
   std::vector<std::string> string_link_addresses;
   base::android::AppendJavaStringArrayToStringVector(env, jlink_addresses,
                                                      &string_link_addresses);
+
   std::vector<net::IPAddress> link_addresses;
   for (std::string const& address : string_link_addresses) {
     link_addresses.push_back(StringToIPAddress(address));
   }
 
   GetTaskRunner()->PostTask(
-      FROM_HERE, base::BindOnce(&HostResolver::SetNetworkAndLinkAddresses,
+      FROM_HERE, base::BindOnce(&HostResolver::SetNetworkLinkAddresses,
                                 base::Unretained(host_resolver_.get()),
-                                net_handle, std::move(link_addresses)));
+                                std::move(link_addresses)));
 }
 
-static jlong JNI_AwPacProcessor_CreateNativePacProcessor(JNIEnv* env) {
-  AwPacProcessor* processor = new AwPacProcessor();
+static jlong JNI_AwPacProcessor_CreateNativePacProcessor(JNIEnv* env,
+                                                         jlong net_handle) {
+  AwPacProcessor* processor = new AwPacProcessor(net_handle);
   return reinterpret_cast<intptr_t>(processor);
 }
 

android_webview/browser/aw_pac_processor.h

@@ -22,7 +22,7 @@ class HostResolver;
 
 class AwPacProcessor {
  public:
-  AwPacProcessor();
+  AwPacProcessor(net_handle_t net_handle);
   AwPacProcessor(const AwPacProcessor&) = delete;
   AwPacProcessor& operator=(const AwPacProcessor&) = delete;
 
@@ -39,9 +39,8 @@ class AwPacProcessor {
       const base::android::JavaParamRef<jobject>& obj,
       const base::android::JavaParamRef<jstring>& jurl);
   std::string MakeProxyRequest(std::string url);
-  void SetNetworkAndLinkAddresses(
+  void SetNetworkLinkAddresses(
       JNIEnv* env,
-      net_handle_t net_handle,
       const base::android::JavaParamRef<jobjectArray>& addresses);
 
  private:
@@ -64,6 +63,7 @@ class AwPacProcessor {
   std::unique_ptr<HostResolver> host_resolver_;
 
   std::set<Job*> jobs_;
+  net_handle_t net_handle_;
 };
 }  // namespace android_webview
 

android_webview/browser/aw_pac_processor_unittest.cc

@@ -33,7 +33,7 @@ class AwPacProcessorTest : public testing::Test {
  protected:
   base::test::TaskEnvironment task_environment_{
            base::test::TaskEnvironment::TimeSource::MOCK_TIME};
-  AwPacProcessor* pac_processor_ = new AwPacProcessor();
+  AwPacProcessor* pac_processor_ = new AwPacProcessor(NETWORK_UNSPECIFIED);
 };
 
 TEST_F(AwPacProcessorTest, MakeProxyRequest) {
@@ -49,7 +49,8 @@ TEST_F(AwPacProcessorTest, MakeProxyRequestDnsResolve) {
 }
 
 TEST_F(AwPacProcessorTest, MultipleProxyRequest) {
-  AwPacProcessor* other_pac_processor_ = new AwPacProcessor();
+  AwPacProcessor* other_pac_processor_ =
+      new AwPacProcessor(NETWORK_UNSPECIFIED);
   pac_processor_->SetProxyScript(kScript);
   other_pac_processor_->SetProxyScript(kScriptDnsResolve);
 

android_webview/java/src/org/chromium/android_webview/AwPacProcessor.java

@@ -24,62 +24,57 @@ import org.chromium.base.annotations.NativeMethods;
 public class AwPacProcessor {
     private long mNativePacProcessor;
     private Network mNetwork;
-    private ConnectivityManager.NetworkCallback mNetworkCallback;
 
     public static final long NETWORK_UNSPECIFIED = 0;
 
     private static class LazyHolder {
-        static final AwPacProcessor sInstance = new AwPacProcessor();
+        static final AwPacProcessor sInstance = new AwPacProcessor(null);
     }
 
     public static AwPacProcessor getInstance() {
         return LazyHolder.sInstance;
     }
 
-    public AwPacProcessor() {
-        mNativePacProcessor = AwPacProcessorJni.get().createNativePacProcessor();
-        registerNetworkCallback();
-    }
-
-    private static ConnectivityManager getConnectivityManager() {
-        Context context = ContextUtils.getApplicationContext();
-        return (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
-    }
-
-    private void updateNetworkLinkAddress(Network network, LinkProperties linkProperties) {
-        if (network == null || linkProperties == null) {
-            setNetworkAndLinkAddresses(NETWORK_UNSPECIFIED, new String[0]);
-        } else {
-            String[] addresses = linkProperties.getLinkAddresses()
-                                         .stream()
-                                         .map(LinkAddress::toString)
-                                         .toArray(String[] ::new);
-            setNetworkAndLinkAddresses(network.getNetworkHandle(), addresses);
+    public AwPacProcessor(Network network) {
+        if (network == null) {
+            mNativePacProcessor =
+                    AwPacProcessorJni.get().createNativePacProcessor(NETWORK_UNSPECIFIED);
+            return;
         }
-    }
 
-    public void setNetworkAndLinkAddresses(long networkHandle, String[] addresses) {
-        AwPacProcessorJni.get().setNetworkAndLinkAddresses(
-                mNativePacProcessor, networkHandle, addresses);
-    }
+        mNetwork = network;
+        mNativePacProcessor =
+                AwPacProcessorJni.get().createNativePacProcessor(mNetwork.getNetworkHandle());
 
-    private void registerNetworkCallback() {
-        mNetworkCallback = new ConnectivityManager.NetworkCallback() {
-            @Override
-            public void onLinkPropertiesChanged(Network network, LinkProperties linkProperties) {
-                if (network.equals(mNetwork)) {
-                    updateNetworkLinkAddress(network, linkProperties);
-                }
-            }
-        };
+        Context context = ContextUtils.getApplicationContext();
+        ConnectivityManager connectivityManager =
+                (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
         NetworkRequest.Builder builder = new NetworkRequest.Builder();
 
-        getConnectivityManager().registerNetworkCallback(builder.build(), mNetworkCallback);
+        connectivityManager.registerNetworkCallback(
+                builder.build(), new ConnectivityManager.NetworkCallback() {
+                    @Override
+                    public void onLinkPropertiesChanged(
+                            Network network, LinkProperties linkProperties) {
+                        if (network.equals(mNetwork)) {
+                            updateNetworkLinkAddress(linkProperties);
+                        }
+                    }
+                });
+
+        updateNetworkLinkAddress(connectivityManager.getLinkProperties(mNetwork));
+    }
+
+    private void updateNetworkLinkAddress(LinkProperties linkProperties) {
+        String[] addresses = linkProperties.getLinkAddresses()
+                                     .stream()
+                                     .map(LinkAddress::toString)
+                                     .toArray(String[] ::new);
+        AwPacProcessorJni.get().setNetworkLinkAddresses(mNativePacProcessor, addresses);
     }
 
     // The calling code must not call any methods after it called destroy().
     public void destroy() {
-        getConnectivityManager().unregisterNetworkCallback(mNetworkCallback);
         AwPacProcessorJni.get().destroyNative(mNativePacProcessor, this);
     }
 
@@ -91,11 +86,6 @@ public class AwPacProcessor {
         return AwPacProcessorJni.get().makeProxyRequest(mNativePacProcessor, this, url);
     }
 
-    public void setNetwork(Network network) {
-        updateNetworkLinkAddress(network, getConnectivityManager().getLinkProperties(network));
-        mNetwork = network;
-    }
-
     public Network getNetwork() {
         return mNetwork;
     }
@@ -107,11 +97,10 @@ public class AwPacProcessor {
     @NativeMethods
     interface Natives {
         void initializeEnvironment();
-        long createNativePacProcessor();
+        long createNativePacProcessor(long netHandle);
         boolean setProxyScript(long nativeAwPacProcessor, AwPacProcessor caller, String script);
         String makeProxyRequest(long nativeAwPacProcessor, AwPacProcessor caller, String url);
         void destroyNative(long nativeAwPacProcessor, AwPacProcessor caller);
-        void setNetworkAndLinkAddresses(
-                long nativeAwPacProcessor, long networkHandle, String[] adresses);
+        void setNetworkLinkAddresses(long nativeAwPacProcessor, String[] adresses);
     }
 }

android_webview/javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java

-// Copyright 2020 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.
-
-package org.chromium.android_webview.test;
-import androidx.test.filters.SmallTest;
-
-import com.android.webview.chromium.WebViewChromiumFactoryProvider;
-
-import org.junit.Assert;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-
-import org.chromium.android_webview.AwPacProcessor;
-import org.chromium.base.JNIUtils;
-import org.chromium.base.library_loader.LibraryLoader;
-
-/**
- * Tests for AwPacProcessor class.
- */
-@RunWith(AwJUnit4ClassRunner.class)
-public class AwPacProcessorTest {
-    private AwPacProcessor mProcessor;
-
-    private final String mPacScript = "function FindProxyForURL(url, host) {\n"
-            + "var x = myIpAddress();"
-            + "\treturn \"PROXY \" + x + \":80\";\n"
-            + "}";
-    private final String mTestUrl = "http://testurl.test";
-
-    @Rule
-    public AwActivityTestRule mRule = new AwActivityTestRule();
-
-    @Before
-    public void setUp() {
-        JNIUtils.setClassLoader(WebViewChromiumFactoryProvider.class.getClassLoader());
-        LibraryLoader.getInstance().ensureInitialized();
-
-        mProcessor = AwPacProcessor.getInstance();
-    }
-
-    @Test
-    @SmallTest
-    public void testUpdateNetworkAndLinkAddresses() throws Throwable {
-        // PAC script returns result of myIpAddress call
-        mProcessor.setProxyScript(mPacScript);
-
-        // Save the proxy request result when network is not set
-        String proxyResultNetworkIsNotSet = mProcessor.makeProxyRequest(mTestUrl);
-
-        // Set network and IP addresses, check they are correctly propagated.
-        mProcessor.setNetworkAndLinkAddresses(42, new String[] {"1.2.3.4"});
-        String proxyResultNetworkIsSet = mProcessor.makeProxyRequest(mTestUrl);
-        Assert.assertEquals("PROXY 1.2.3.4:80", proxyResultNetworkIsSet);
-
-        // Unset network, the returned proxy string must be equal previously saved value
-        mProcessor.setNetwork(null);
-        Assert.assertEquals(proxyResultNetworkIsNotSet, mProcessor.makeProxyRequest(mTestUrl));
-    }
-}
\ No newline at end of file

android_webview/test/BUILD.gn

@@ -255,7 +255,6 @@ instrumentation_test_apk("webview_instrumentation_test_apk") {
     "../javatests/src/org/chromium/android_webview/test/AwLegacyQuirksTest.java",
     "../javatests/src/org/chromium/android_webview/test/AwMetricsIntegrationTest.java",
     "../javatests/src/org/chromium/android_webview/test/AwNetworkConfigurationTest.java",
-    "../javatests/src/org/chromium/android_webview/test/AwPacProcessorTest.java",
     "../javatests/src/org/chromium/android_webview/test/AwPageLoadMetricsTest.java",
     "../javatests/src/org/chromium/android_webview/test/AwPermissionManagerTest.java",
     "../javatests/src/org/chromium/android_webview/test/AwProxyControllerTest.java",