Bug 1289242 - Avoid unchecked warning and check for correct type when casting EnumSet r?sebastian draft
authorAndrzej Hunt <ahunt@mozilla.com>
Mon, 01 Aug 2016 08:49:01 -0700
changeset 399221 154b12b90e68d57a1b70edc570ea62a378bb9ffb
parent 399220 003f241995dacb66aa20de8ac1c996da224702d5
child 527869 edd3f5de679d58c19bb4ff8d14987cecc7a064ed
push id25764
push userahunt@mozilla.com
push dateWed, 10 Aug 2016 17:18:33 +0000
reviewerssebastian
bugs1289242
milestone51.0a1
Bug 1289242 - Avoid unchecked warning and check for correct type when casting EnumSet r?sebastian Yes this is rather horrible. MozReview-Commit-ID: Hu5diYVmlv1
mobile/android/base/java/org/mozilla/gecko/home/activitystream/ASOpenURLDelegate.java
mobile/android/tests/background/junit4/src/org/mozilla/gecko/home/activitystream/TestFlagDeserialization.java
--- a/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ASOpenURLDelegate.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/activitystream/ASOpenURLDelegate.java
@@ -63,24 +63,65 @@ public class ASOpenURLDelegate extends B
         Bundle flagsBundle = new Bundle();
         flagsBundle.putSerializable(KEY_FLAGS, flags);
         data.putExtra(KEY_FLAGS, flagsBundle);
 
         activity.setResult(BrowserApp.ACTIVITY_RESULT_AS_DETAIL_OPEN, data);
         activity.finish();
     }
 
+    @SuppressWarnings("unchecked")
+    static EnumSet<HomePager.OnUrlOpenListener.Flags> castEnumSetToHomePagerFlags(EnumSet<?> in) {
+        // Bundle's only return a Serializable, whereas we're trying to obtain the actual
+        // Enumset. Casting to a generic type results in unchecked warnings, which we need to suppress
+        // here (there is unfortunately no way around this). We use a separate method to minimise
+        // the volume of code with suppressed warnings (warning supressions can't be applied to single
+        // lines of code, or even to blocks, but they can be applied to methods). This also gives
+        // us the opportunity to verify that we have the correct type parameter. Not checking the type
+        // means we could run into ClassCastExceptions the first time we try to access values
+        // in the EnumSet - without any hint or warning of the Exceptions being likely since
+        // at that point we think we already have the correct type.
+
+        // This first cast does esentially nothing. The compiler now thinks we have the expected type,
+        // but this is unsafe because the type parameter of <code>in</code> is not verified at any point
+        // (hence the unchecked warning suppression above).
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flags =  (EnumSet<HomePager.OnUrlOpenListener.Flags>) in;
+
+        // Fortunately, the EnumSet implementation does store the type parameter's class, and
+        // we can use that to have an (implementation-dependent) test of the type parameter.
+        // The EnumSet tests that the input flag is of the correct type during various operations
+        // such as comparison and insertion/removal.
+        // One way of doing this is to check whether either the current EnumSet, or it's complement,
+        // contain a given flag. contains() returns false either if the Flag isn't contained, or if
+        // the Flag in question is of a different type to that of the EnumSet. If both the EnumSet
+        // and it's complement return false then we know that we have a different type. (Similar variations
+        // are possible by testing both add() and remove() - either one or the other returns true if the Flag
+        // is of the correct type.)
+
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flagsComplement = EnumSet.complementOf(flags);
+
+        final HomePager.OnUrlOpenListener.Flags testFlag = HomePager.OnUrlOpenListener.Flags.ALLOW_SWITCH_TO_TAB;
+        if (flags.contains(testFlag) ^ flagsComplement.contains(testFlag)) {
+            // One of the two sets contains our test flag - we have the right type:
+            return flags;
+        } else {
+            throw new ClassCastException("Flags EnumSet contains wrong parameter");
+        }
+
+    }
+
     @Override
     public void onActivityResult(BrowserApp browserApp, int requestCode, int resultCode,
                                  Intent data) {
         if (requestCode == BrowserApp.ACTIVITY_REQUEST_AS_DETAIL &&
             resultCode == BrowserApp.ACTIVITY_RESULT_AS_DETAIL_OPEN) {
             final String url = data.getStringExtra(KEY_URL);
             final Bundle flagsBundle = data.getBundleExtra(KEY_FLAGS);
-            final EnumSet<HomePager.OnUrlOpenListener.Flags> flags = (EnumSet<HomePager.OnUrlOpenListener.Flags>) flagsBundle.getSerializable(KEY_FLAGS);
+
+            final EnumSet<HomePager.OnUrlOpenListener.Flags> flags = castEnumSetToHomePagerFlags((EnumSet<?>) flagsBundle.getSerializable(KEY_FLAGS));
 
             browserApp.onUrlOpen(url, flags);
         }
     }
 
     @Override
     public void onCreate(BrowserApp browserApp, Bundle savedInstanceState) {
         // Early return: do nothing if AS is disabled
new file mode 100644
--- /dev/null
+++ b/mobile/android/tests/background/junit4/src/org/mozilla/gecko/home/activitystream/TestFlagDeserialization.java
@@ -0,0 +1,45 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 20; indent-tabs-mode: nil; -*-
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+package org.mozilla.gecko.home.activitystream;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mozilla.gecko.background.testhelpers.TestRunner;
+import org.mozilla.gecko.db.BrowserDB;
+import org.mozilla.gecko.home.HomePager;
+
+import java.util.EnumSet;
+
+@RunWith(TestRunner.class)
+public class TestFlagDeserialization {
+
+    @Test
+    public void testWorkingFlagsNone() {
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flags = EnumSet.noneOf(HomePager.OnUrlOpenListener.Flags.class);
+
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flagsOut = ASOpenURLDelegate.castEnumSetToHomePagerFlags(flags);
+
+        Assert.assertEquals(flags, flagsOut);
+    }
+
+    @Test
+    public void testWorkingFlagsOne() {
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flags = EnumSet.of(HomePager.OnUrlOpenListener.Flags.ALLOW_SWITCH_TO_TAB);
+
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flagsOut = ASOpenURLDelegate.castEnumSetToHomePagerFlags(flags);
+
+        Assert.assertEquals(flags, flagsOut);
+    }
+
+    @Test(expected=ClassCastException.class)
+    public void testFlagsWrongParameter() {
+        // Some random unrelated flags
+        final EnumSet<BrowserDB.FilterFlags> flags = EnumSet.noneOf(BrowserDB.FilterFlags.class);
+
+        // This should throw a ClassCastException
+        final EnumSet<HomePager.OnUrlOpenListener.Flags> flagsOut = ASOpenURLDelegate.castEnumSetToHomePagerFlags(flags);
+    }
+}