Remove usage of the generic multiple permissions update string.

chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java

Index: chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java b/chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java
index a153cb4a9c5754a0a040808175a3042acbdc44f9..304bf593a90327c79655d64163eeeaf4e71cb193 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/permissions/AndroidPermissionRequester.java
@@ -20,6 +20,7 @@ import org.chromium.ui.base.WindowAndroid;
 import org.chromium.ui.base.WindowAndroid.PermissionCallback;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
@@ -84,7 +85,26 @@ public class AndroidPermissionRequester {
             return R.string.infobar_missing_camera_permission_text;
         }
         assert false : "Unexpected content setting type received: " + contentSettingsType;
-        return R.string.infobar_missing_multiple_permissions_text;
+        return -1;
+    }
+
+    private static int updateDeniedPermissionResourceId(int currentDeniedStringId, int deniedCount,
+            SparseArray<String[]> contentSettingsTypesToPermissionsMap, String permission) {
+        if (deniedCount > 1) {
+            int newDeniedStringId =
+                    getDeniedPermissionResourceId(contentSettingsTypesToPermissionsMap, permission);
+            if (newDeniedStringId == currentDeniedStringId) return currentDeniedStringId;
+
+            Set<Integer> ids =
+                    new HashSet<Integer>(Arrays.asList(currentDeniedStringId, newDeniedStringId));
+            if (ids.contains(R.string.infobar_missing_camera_permission_text)
+                    && ids.contains(R.string.infobar_missing_microphone_permission_text)) {
+                return R.string.infobar_missing_microphone_camera_permissions_text;
+            }
+            assert false : "Unexpected combination of content settings types.";
+            return -1;
+        }
+        return getDeniedPermissionResourceId(contentSettingsTypesToPermissionsMap, permission);

[raymes, 2017/05/14 23:22:24]

A few optional suggestions:

private static int updateDeniedPermissionResourceId(...) {
  int newDeniedStringId = ...;

  if (deniedCount == 1)
    return newDeniedStringId;

  boolean deniedCamera = 
      currentDeniedStringId == R.string.infobar_missing_camera_permission_text
|| 
      newDeniedStringId == R.string.infobar_missing_camera_permission_text;

  boolean deniedMic = 
      currentDeniedStringId ==
R.string.infobar_missing_microphone_permission_text || 
      newDeniedStringId == R.string.infobar_missing_microphone_permission_text;

  if (deniedMic && deniedCamera)
    return R.string.infobar_missing_microphone_camera_permissions_text;

  assert false : ...;
  return -1;
}

[benwells, 2017/05/16 15:21:30]

I've checked for ==1 first. I need to keep the check if the two strings are
equal, because if it is location that is denied it will come in twice with the
same string returned each time.

For checking that in the other case it is only mic and camera, I have a slight
preference to use the hash set because it is less repetition of long constants /
a bit more readable. Do you think it is important?

[raymes, 2017/05/17 01:21:43]

It's not important :) I think it just comes down to preference (that's why I
listed them as optional suggestions)

     }
 
     /**
@@ -106,20 +126,15 @@ public class AndroidPermissionRequester {
 
         PermissionCallback callback = new PermissionCallback() {
             @Override
-            public void onRequestPermissionsResult(
-                    String[] permissions, int[] grantResults) {
+            public void onRequestPermissionsResult(String[] permissions, int[] grantResults) {
                 int deniedCount = 0;
                 int requestableCount = 0;
-                int deniedStringId = R.string.infobar_missing_multiple_permissions_text;
+                int deniedStringId = -1;
                 for (int i = 0; i < grantResults.length; i++) {
                     if (grantResults[i] == PackageManager.PERMISSION_DENIED) {
                         deniedCount++;
-                        if (deniedCount > 1) {
-                            deniedStringId = R.string.infobar_missing_multiple_permissions_text;
-                        } else {
-                            deniedStringId = getDeniedPermissionResourceId(
-                                    contentSettingsTypesToPermissionsMap, permissions[i]);
-                        }
+                        deniedStringId = updateDeniedPermissionResourceId(deniedStringId,
+                                deniedCount, contentSettingsTypesToPermissionsMap, permissions[i]);
 
                         if (windowAndroid.canRequestPermission(permissions[i])) {
                             requestableCount++;
@@ -128,7 +143,8 @@ public class AndroidPermissionRequester {
                 }
 
                 Activity activity = windowAndroid.getActivity().get();
-                if (deniedCount > 0 && requestableCount > 0 && activity != null) {
+                if (deniedCount > 0 && deniedStringId != -1 && requestableCount > 0

[raymes, 2017/05/14 23:22:24]

Could this happen?

[benwells, 2017/05/16 15:21:30]

Could this if be true? Or could deniedStringId == -1?

For the first question, yes. If the user says no to the chrome permission, this
block then shows the infobar again, and if the user says "Update permissions"
will ask for chrome permission again. It will keep doing this in a loop until
the user doesn't say "Update permissions" or says yes to the chrome permission.

For the second question, no it shouldn't happen. But if an error is ever
introduced in calling code, it might and we only have the asserts to stop the
code otherwise.

[raymes, 2017/05/17 01:21:43]

Sorry I should have been more clear :) I was only really asking if
deniedStringId could == -1. What is the benefit of not taking this codepath if
it turns out to be -1? I guess I feel that in either case there is a bug in the
code and that it might be more obvious if it crashes or displays some nonsense?
It also just makes the code slightly simpler. What do you think?

+                        && activity != null) {
                     View view = activity.getLayoutInflater().inflate(
                             R.layout.update_permissions_dialog, null);
                     TextView dialogText = (TextView) view.findViewById(R.id.text);