Do screenshot capture async for share intents.

chrome/android/java/src/org/chromium/chrome/browser/ChromeActivity.java

 // 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.
 
 package org.chromium.chrome.browser;
 
 import android.annotation.SuppressLint;
 import android.annotation.TargetApi;
 import android.app.Activity;
 import android.app.SearchManager;
 import android.app.assist.AssistContent;
 import android.content.Context;
 import android.content.Intent;
 import android.content.res.Configuration;
 import android.graphics.Bitmap;
 import android.graphics.Color;
 import android.graphics.Rect;
 import android.graphics.drawable.ColorDrawable;
 import android.graphics.drawable.Drawable;
 import android.net.Uri;
+import android.os.AsyncTask;
 import android.os.Build;
 import android.os.Bundle;
 import android.os.Looper;
 import android.os.MessageQueue;
 import android.os.SystemClock;
 import android.support.v7.app.AlertDialog;
 import android.util.DisplayMetrics;
 import android.view.Menu;
 import android.view.MenuItem;
 import android.view.View;
@@ skipping 85 matching lines @@
 import org.chromium.chrome.browser.util.FeatureUtilities;
 import org.chromium.chrome.browser.webapps.AddToHomescreenDialog;
 import org.chromium.chrome.browser.widget.ControlContainer;
 import org.chromium.components.bookmarks.BookmarkId;
 import org.chromium.content.browser.ContentVideoView;
 import org.chromium.content.browser.ContentViewCore;
 import org.chromium.content.common.ContentSwitches;
 import org.chromium.content_public.browser.ContentBitmapCallback;
 import org.chromium.content_public.browser.LoadUrlParams;
 import org.chromium.content_public.browser.WebContents;
-import org.chromium.content_public.browser.readback_types.ReadbackResponse;
 import org.chromium.policy.CombinedPolicyProvider.PolicyChangeListener;
 import org.chromium.printing.PrintManagerDelegateImpl;
 import org.chromium.printing.PrintingController;
 import org.chromium.ui.base.ActivityWindowAndroid;
 import org.chromium.ui.base.DeviceFormFactor;
 import org.chromium.ui.base.PageTransition;
 import org.chromium.ui.base.WindowAndroid;
 
 import java.util.ArrayList;
 import java.util.List;
@@ skipping 810 matching lines @@
         return super.onOptionsItemSelected(item);
     }
 
     /**
      * Triggered when the share menu item is selected.
      * This creates and shows a share intent picker dialog or starts a share intent directly.
      * @param shareDirectly Whether it should share directly with the activity that was most
      *                      recently used to share.
      * @param isIncognito Whether currentTab is incognito.
      */
     public void onShareMenuItemSelected(final boolean shareDirectly, boolean isIncognito) {

[nyquist, 2016/07/14 17:36:32]

Was there no test you needed to update for this?
If so, could you please add one?

[ssid, 2016/07/19 18:32:57]

Done.

         final Tab currentTab = getActivityTab();
         if (currentTab == null) return;
 
         final Activity mainActivity = this;
-        ContentBitmapCallback callback = new ContentBitmapCallback() {
-                    @Override
-                    public void onFinishGetBitmap(Bitmap bitmap, int response) {
-                        // Check whether this page is an offline page, and use its online URL if so.
-                        String url = currentTab.getOfflinePageOriginalUrl();
-                        RecordHistogram.recordBooleanHistogram(
-                                "OfflinePages.SharedPageWasOffline", url != null);
 
-                        // If there is no entry in the offline pages DB for this tab, use the tab's
-                        // URL directly.
-                        if (url == null) url = currentTab.getUrl();
+        // Check whether this page is an offline page, and use its online URL if so.
+        String url = currentTab.getOfflinePageOriginalUrl();
+        RecordHistogram.recordBooleanHistogram("OfflinePages.SharedPageWasOffline", url != null);
+        // If there is no entry in the offline pages DB for this tab, use the tab's URL directly.
+        if (url == null) url = currentTab.getUrl();
 
-                        ShareHelper.share(
-                                shareDirectly, mainActivity, currentTab.getTitle(), url, bitmap);
-                        if (shareDirectly) {
-                            RecordUserAction.record("MobileMenuDirectShare");
-                        } else {
-                            RecordUserAction.record("MobileMenuShare");
-                        }
-                    }
-                };
+        if (shareDirectly) {
+            RecordUserAction.record("MobileMenuDirectShare");
+        } else {
+            RecordUserAction.record("MobileMenuShare");
+        }
+
         if (isIncognito || currentTab.getWebContents() == null) {
-            callback.onFinishGetBitmap(null, ReadbackResponse.SURFACE_UNAVAILABLE);
+            ShareHelper.share(shareDirectly, mainActivity, currentTab.getTitle(), url, null);
         } else {
-            currentTab.getWebContents().getContentBitmapAsync(
-                    Bitmap.Config.ARGB_8888, 1.f, EMPTY_RECT, callback);
+            // Share an empty uuid in place of screenshot file.
+            final String fileName = String.valueOf(System.currentTimeMillis());

[nyquist, 2016/07/14 17:36:32]

Will the user ever see this filename? If not, maybe we could use something that
won't ever overlap? I was thinking about for example daylight savings time. It's
a weird case though, but if it's not user-visible, maybe elapsedRealtime() or
maybe even elapsedRealtimeNanos()?

[ssid, 2016/07/19 18:32:57]

The uri wouldn't overlap with anything because the name would be something like
"content://com.google.android.apps.chrome.BlockingFileProvider/1468532221978".
So this will be unique across chromium since it has "BlockingFileProvider".

The actual file itself has an uri like
"content://com.android.chrome.FileProvider/images/screenshot/14642960919551746825363.jpg"
This is exactly the same name and folder used previous to the change and I just
kept it. Do you want to change this ? How about to "chrome-screenshot-%d"?

[nyquist, 2016/07/29 23:45:27]

That'd be OK too. I was just thinking about if you take a screenshot just before
daylight savings time, and then again exactly an hour (on the millisecond)
later. Or immediately after each other. I don't think that'll happen in practice
though. Feel free to keep as is too.

[ssid, 2016/08/03 22:33:40]

The description says it returns the difference between current time and
midnight, January 1, 1970 UTC. I think this will be unique?

+            final Uri uuid = BlockingFileProvider.getContentUriForFile(mainActivity, fileName);
+            ShareHelper.share(shareDirectly, mainActivity, currentTab.getTitle(), url, uuid);
+
+            // Start screenshot capture async and notify the provider when it is ready.
+            final ContentBitmapCallback callback = new ContentBitmapCallback() {
+                @Override
+                public void onFinishGetBitmap(Bitmap bitmap, int response) {
+                    ShareHelper.onScreenshotReady(fileName, bitmap, mainActivity);

[nyquist, 2016/07/14 17:36:32]

Is mainActivity guaranteed to be alive here, or will
ApplicationState.HAS_DESTROYED_ACTIVITIES catch that in the ShareHelper?

[ssid, 2016/07/19 18:32:57]

The check at the end of onScreenshotready should take care of activity being
alive. I don't think that other code path that sends the intent needs this check
since it is creating intent synchronously.

+                }
+            };
+            new AsyncTask<Void, Void, Void>() {
+                @Override
+                protected Void doInBackground(Void... params) {
+                    currentTab.getWebContents().getContentBitmapAsync(

[nyquist, 2016/07/14 17:36:32]

Is it guaranteed that getWebContents() does not return null at this point?

[ssid, 2016/07/19 18:32:57]

added a check

+                            Bitmap.Config.ARGB_8888, 1.f, EMPTY_RECT, callback);
+                    return null;
+                }
+            }.execute();
         }
     }
 
     /**
      * @return Whether the activity is in overview mode.
      */
     public boolean isInOverviewMode() {
         return false;
     }
 
@@ skipping 747 matching lines @@
                 "Android.MultiWindowMode.IsTabletScreenWidthBelow600",
                 mScreenWidthDp < DeviceFormFactor.MINIMUM_TABLET_WIDTH_DP);
 
         if (mScreenWidthDp < DeviceFormFactor.MINIMUM_TABLET_WIDTH_DP) {
             RecordHistogram.recordLinearCountHistogram(
                     "Android.MultiWindowMode.TabletScreenWidth", mScreenWidthDp, 1,
                     DeviceFormFactor.MINIMUM_TABLET_WIDTH_DP, 50);
         }
     }
 }