Bug 1122302 - Handle reusing the share overlay. r=rnewman
authorMichael Comella <michael.l.comella@gmail.com>
Thu, 12 Mar 2015 11:59:35 -0700
changeset 233737 a8ff0289a76319a7d9ed5182b87af439b2b40782
parent 233736 13fa33b1876ec63043249589c44ddc2c1a0080f7
child 233738 cac8ee9abe860889c7bd686ac872453fbf1a9c8d
push id28423
push userphilringnalda@gmail.com
push dateMon, 16 Mar 2015 02:35:36 +0000
treeherdermozilla-central@436686833af0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs1122302, 1137928
milestone39.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1122302 - Handle reusing the share overlay. r=rnewman Ideally, we'd want to create a new instance of the share overlay on each call but Android L changes the behavior and forces reuse (bug 1137928). Ugh.
mobile/android/base/AndroidManifest.xml.in
mobile/android/base/overlays/ui/ShareDialog.java
--- a/mobile/android/base/AndroidManifest.xml.in
+++ b/mobile/android/base/AndroidManifest.xml.in
@@ -373,21 +373,25 @@
                   android:permission="@ANDROID_PACKAGE_NAME@.permissions.BROWSER_PROVIDER">
 
             <path-permission android:pathPrefix="/search_suggest_query"
                              android:readPermission="android.permission.GLOBAL_SEARCH" />
 
         </provider>
 
 #ifdef MOZ_ANDROID_SHARE_OVERLAY
-        <!-- Share overlay activity -->
+        <!-- Share overlay activity
+
+             Setting launchMode="singleTop" ensure onNewIntent is called when the Activity is
+             reused. Ideally we create a new instance but Android L breaks this (bug 1137928). -->
         <activity android:name="org.mozilla.gecko.overlays.ui.ShareDialog"
                   android:label="@string/overlay_share_label"
                   android:theme="@style/ShareOverlayActivity"
                   android:configChanges="keyboard|keyboardHidden|mcc|mnc|locale|layoutDirection"
+                  android:launchMode="singleTop"
                   android:windowSoftInputMode="stateAlwaysHidden|adjustResize">
 
             <intent-filter>
                 <action android:name="android.intent.action.SEND" />
                 <category android:name="android.intent.category.DEFAULT" />
                 <data android:mimeType="text/plain" />
             </intent-filter>
 
--- a/mobile/android/base/overlays/ui/ShareDialog.java
+++ b/mobile/android/base/overlays/ui/ShareDialog.java
@@ -59,17 +59,21 @@ public class ShareDialog extends Locales
     private static final String LOGTAG = "GeckoShareDialog";
 
     /** Flag that we should only show the devices for send tab in this dialog. **/
     public static final String INTENT_EXTRA_DEVICES_ONLY = "org.mozilla.gecko.intent.extra.DEVICES_ONLY";
 
     /** The maximum number of devices we'll show in the dialog when in State.DEFAULT. **/
     private static final int MAXIMUM_INLINE_DEVICES = 2;
 
-    private State state = State.DEFAULT;
+    private State state;
+
+    private SendTabList sendTabList;
+    private OverlayDialogButton readingListButton;
+    private OverlayDialogButton bookmarkButton;
 
     private String url;
     private String title;
 
     // The override intent specified by SendTab (if any). See SendTab.java.
     private Intent sendTabOverrideIntent;
 
     // Flag set during animation to prevent animation multiple-start.
@@ -91,34 +95,32 @@ public class ShareDialog extends Locales
     };
 
     /**
      * Called when a UI event broadcast is received from the SendTab ShareMethod.
      */
     protected void handleSendTabUIEvent(Intent intent) {
         sendTabOverrideIntent = intent.getParcelableExtra(SendTab.OVERRIDE_INTENT);
 
-        SendTabList sendTabList = (SendTabList) findViewById(R.id.overlay_send_tab_btn);
-
         ParcelableClientRecord[] clientrecords = (ParcelableClientRecord[]) intent.getParcelableArrayExtra(SendTab.EXTRA_CLIENT_RECORDS);
 
         // Escape hatch: we don't show the option to open this dialog in this state so this should
         // never be run. However, due to potential inconsistencies in synced client state
         // (e.g. bug 1122302 comment 47), we might fail.
         if (state == State.DEVICES_ONLY && clientrecords.length == 0) {
             Log.e(LOGTAG, "In state: " + State.DEVICES_ONLY + " and received 0 synced clients. Finishing...");
             Toast.makeText(this, getResources().getText(R.string.overlay_no_synced_devices), Toast.LENGTH_SHORT)
                  .show();
             finish();
             return;
         }
 
         sendTabList.setSyncClients(clientrecords);
 
-        if (state == State.DEVICES_ONLY || sendTabList.getCount() <= MAXIMUM_INLINE_DEVICES) {
+        if (state == State.DEVICES_ONLY || clientrecords.length <= MAXIMUM_INLINE_DEVICES) {
             // Show the list of devices in-line.
             sendTabList.switchState(SendTabList.State.LIST);
             return;
         }
 
         // Just show a button to launch the list of devices to choose from.
         sendTabList.switchState(SendTabList.State.SHOW_DEVICES);
     }
@@ -147,152 +149,168 @@ public class ShareDialog extends Locales
         finish();
     }
 
     @Override
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
 
         getWindow().setWindowAnimations(0);
+        setContentView(R.layout.overlay_share_dialog);
 
-        Intent intent = getIntent();
+        LocalBroadcastManager.getInstance(this).registerReceiver(uiEventListener,
+                new IntentFilter(OverlayConstants.SHARE_METHOD_UI_EVENT));
+
+        // Send tab.
+        sendTabList = (SendTabList) findViewById(R.id.overlay_send_tab_btn);
+
+        // Register ourselves as both the listener and the context for the Adapter.
+        final SendTabDeviceListArrayAdapter adapter = new SendTabDeviceListArrayAdapter(this, this);
+        sendTabList.setAdapter(adapter);
+        sendTabList.setSendTabTargetSelectedListener(this);
+
+        bookmarkButton = (OverlayDialogButton) findViewById(R.id.overlay_share_bookmark_btn);
+        readingListButton = (OverlayDialogButton) findViewById(R.id.overlay_share_reading_list_btn);
+
         final Resources resources = getResources();
+        final String bookmarkEnabledLabel = resources.getString(R.string.overlay_share_bookmark_btn_label);
+        final Drawable bookmarkEnabledIcon = resources.getDrawable(R.drawable.overlay_bookmark_icon);
+        bookmarkButton.setEnabledLabelAndIcon(bookmarkEnabledLabel, bookmarkEnabledIcon);
+
+        final String bookmarkDisabledLabel = resources.getString(R.string.overlay_share_bookmark_btn_label_already);
+        final Drawable bookmarkDisabledIcon = resources.getDrawable(R.drawable.overlay_bookmarked_already_icon);
+        bookmarkButton.setDisabledLabelAndIcon(bookmarkDisabledLabel, bookmarkDisabledIcon);
+
+        bookmarkButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View view) {
+                addBookmark();
+            }
+        });
+
+        final String readingListEnabledLabel = resources.getString(R.string.overlay_share_reading_list_btn_label);
+        final Drawable readingListEnabledIcon = resources.getDrawable(R.drawable.overlay_readinglist_icon);
+        readingListButton.setEnabledLabelAndIcon(readingListEnabledLabel, readingListEnabledIcon);
+
+        final String readingListDisabledLabel = resources.getString(R.string.overlay_share_reading_list_btn_label_already);
+        final Drawable readingListDisabledIcon = resources.getDrawable(R.drawable.overlay_readinglist_already_icon);
+        readingListButton.setDisabledLabelAndIcon(readingListDisabledLabel, readingListDisabledIcon);
+
+        readingListButton.setOnClickListener(new View.OnClickListener() {
+            @Override
+            public void onClick(View view) {
+                addToReadingList();
+            }
+        });
+    }
+
+    @Override
+    protected void onResume() {
+        super.onResume();
+
+        final Intent intent = getIntent();
+
+        state = (intent.getBooleanExtra(INTENT_EXTRA_DEVICES_ONLY, false) ?
+                State.DEVICES_ONLY : State.DEFAULT);
+
+        // If the Activity is being reused, we need to reset the state. Ideally, we create a
+        // new instance for each call, but Android L breaks this (bug 1137928).
+        sendTabList.switchState(SendTabList.State.LOADING);
 
         // The URL is usually hiding somewhere in the extra text. Extract it.
         final String extraText = ContextUtils.getStringExtra(intent, Intent.EXTRA_TEXT);
         if (TextUtils.isEmpty(extraText)) {
             abortDueToNoURL();
             return;
         }
 
         final String pageUrl = new WebURLFinder(extraText).bestWebURL();
         if (TextUtils.isEmpty(pageUrl)) {
             abortDueToNoURL();
             return;
         }
 
-        setContentView(R.layout.overlay_share_dialog);
-
-
-        LocalBroadcastManager.getInstance(this).registerReceiver(uiEventListener,
-                                                                  new IntentFilter(OverlayConstants.SHARE_METHOD_UI_EVENT));
-
         // Have the service start any initialisation work that's necessary for us to show the correct
         // UI. The results of such work will come in via the BroadcastListener.
         Intent serviceStartupIntent = new Intent(this, OverlayActionService.class);
         serviceStartupIntent.setAction(OverlayConstants.ACTION_PREPARE_SHARE);
         startService(serviceStartupIntent);
 
         // If provided, we use the subject text to give us something nice to display.
         // If not, we wing it with the URL.
 
         // TODO: Consider polling Fennec databases to find better information to display.
-        String subjectText = intent.getStringExtra(Intent.EXTRA_SUBJECT);
+        final String subjectText = intent.getStringExtra(Intent.EXTRA_SUBJECT);
 
-        String telemetryExtras = "title=" + (subjectText != null);
+        final String telemetryExtras = "title=" + (subjectText != null);
         if (subjectText != null) {
             ((TextView) findViewById(R.id.title)).setText(subjectText);
         }
 
         Telemetry.sendUIEvent(TelemetryContract.Event.SHOW, TelemetryContract.Method.SHARE_OVERLAY, telemetryExtras);
 
         title = subjectText;
         url = pageUrl;
 
         // Set the subtitle text on the view and cause it to marquee if it's too long (which it will
         // be, since it's a URL).
-        TextView subtitleView = (TextView) findViewById(R.id.subtitle);
+        final TextView subtitleView = (TextView) findViewById(R.id.subtitle);
         subtitleView.setText(pageUrl);
         subtitleView.setEllipsize(TextUtils.TruncateAt.MARQUEE);
         subtitleView.setSingleLine(true);
         subtitleView.setMarqueeRepeatLimit(5);
         subtitleView.setSelected(true);
 
-        // Start the slide-up animation.
-        Animation anim = AnimationUtils.loadAnimation(this, R.anim.overlay_slide_up);
-        findViewById(R.id.sharedialog).startAnimation(anim);
+        final ImageView foxIcon = (ImageView) findViewById(R.id.share_overlay_icon);
+        final LinearLayout topBar = (LinearLayout) findViewById(R.id.share_overlay_top_bar);
+
+        if (state == State.DEVICES_ONLY) {
+            bookmarkButton.setVisibility(View.GONE);
+            readingListButton.setVisibility(View.GONE);
+
+            foxIcon.setOnClickListener(null);
+            topBar.setOnClickListener(null);
+            return;
+        }
+
+        bookmarkButton.setVisibility(View.VISIBLE);
+        readingListButton.setVisibility(View.VISIBLE);
 
         // Configure buttons.
-        final ImageView foxIcon = (ImageView) findViewById(R.id.share_overlay_icon);
-        final LinearLayout topBar = (LinearLayout) findViewById(R.id.share_overlay_top_bar);
-        View.OnClickListener launchBrowser = new View.OnClickListener() {
+        final View.OnClickListener launchBrowser = new View.OnClickListener() {
             @Override
             public void onClick(View view) {
                 ShareDialog.this.launchBrowser();
             }
         };
 
         foxIcon.setOnClickListener(launchBrowser);
         topBar.setOnClickListener(launchBrowser);
 
-        // Send tab.
-        final SendTabList sendTabList = (SendTabList) findViewById(R.id.overlay_send_tab_btn);
-
-        // Register ourselves as both the listener and the context for the Adapter.
-        final SendTabDeviceListArrayAdapter adapter = new SendTabDeviceListArrayAdapter(this, this);
-        sendTabList.setAdapter(adapter);
-        sendTabList.setSendTabTargetSelectedListener(this);
-
-        final OverlayDialogButton bookmarkBtn = (OverlayDialogButton) findViewById(R.id.overlay_share_bookmark_btn);
-        final OverlayDialogButton readinglistBtn = (OverlayDialogButton) findViewById(R.id.overlay_share_reading_list_btn);
-
-        if (intent.getBooleanExtra(INTENT_EXTRA_DEVICES_ONLY, false)) {
-            // Note: we update the device list state after the device list is received. We have to
-            // do it this way because the list doesn't handle switching state while it is loading.
-            state = State.DEVICES_ONLY;
-
-            readinglistBtn.setVisibility(View.GONE);
-            bookmarkBtn.setVisibility(View.GONE);
-            return;
-        }
-
-        final String bookmarkEnabledLabel = resources.getString(R.string.overlay_share_bookmark_btn_label);
-        final Drawable bookmarkEnabledIcon = resources.getDrawable(R.drawable.overlay_bookmark_icon);
-        bookmarkBtn.setEnabledLabelAndIcon(bookmarkEnabledLabel, bookmarkEnabledIcon);
+        final LocalBrowserDB browserDB = new LocalBrowserDB(getCurrentProfile());
+        setButtonState(url, browserDB);
 
-        final String bookmarkDisabledLabel = resources.getString(R.string.overlay_share_bookmark_btn_label_already);
-        final Drawable bookmarkDisabledIcon = resources.getDrawable(R.drawable.overlay_bookmarked_already_icon);
-        bookmarkBtn.setDisabledLabelAndIcon(bookmarkDisabledLabel, bookmarkDisabledIcon);
-
-        bookmarkBtn.setOnClickListener(new View.OnClickListener() {
-            @Override
-            public void onClick(View view) {
-                addBookmark();
-            }
-        });
-
-        final String readingListEnabledLabel = resources.getString(R.string.overlay_share_reading_list_btn_label);
-        final Drawable readingListEnabledIcon = resources.getDrawable(R.drawable.overlay_readinglist_icon);
-        readinglistBtn.setEnabledLabelAndIcon(readingListEnabledLabel, readingListEnabledIcon);
-
-        final String readingListDisabledLabel = resources.getString(R.string.overlay_share_reading_list_btn_label_already);
-        final Drawable readingListDisabledIcon = resources.getDrawable(R.drawable.overlay_readinglist_already_icon);
-        readinglistBtn.setDisabledLabelAndIcon(readingListDisabledLabel, readingListDisabledIcon);
-
-        readinglistBtn.setOnClickListener(new View.OnClickListener() {
-            @Override
-            public void onClick(View view) {
-                addToReadingList();
-            }
-        });
+        // Start the slide-up animation.
+        final Animation anim = AnimationUtils.loadAnimation(this, R.anim.overlay_slide_up);
+        findViewById(R.id.sharedialog).startAnimation(anim);
     }
 
     @Override
-    protected void onResume() {
-        super.onResume();
+    protected void onNewIntent(final Intent intent) {
+        super.onNewIntent(intent);
 
-        LocalBrowserDB browserDB = new LocalBrowserDB(getCurrentProfile());
-        disableButtonsIfAlreadyAdded(url, browserDB);
+        // The intent returned by getIntent is not updated automatically.
+        setIntent(intent);
     }
 
     /**
-     * Disables the bookmark/reading list buttons if the given URL is already in the corresponding
-     * list.
+     * Sets the state of the bookmark/reading list buttons: they are disabled if the given URL is
+     * already in the corresponding list.
      */
-    private void disableButtonsIfAlreadyAdded(final String pageURL, final LocalBrowserDB browserDB) {
+    private void setButtonState(final String pageURL, final LocalBrowserDB browserDB) {
         new UIAsyncTask.WithoutParams<Void>(ThreadUtils.getBackgroundHandler()) {
             // Flags to hold the result
             boolean isBookmark;
             boolean isReadingListItem;
 
             @Override
             protected Void doInBackground() {
                 final ContentResolver contentResolver = getApplicationContext().getContentResolver();