[Android] Show Browser Actions dialog in Chrome

chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java

Index: chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java b/chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java
index cf4f9d21295e58a3b40ced65671764914152986e..2932ee4f3ce876285d3921d72977db150bf8f6bf 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/browseractions/BrowserActionActivity.java
@@ -11,21 +11,27 @@ import android.net.Uri;
 import android.os.Bundle;
 import android.support.customtabs.browseractions.BrowserActionItem;
 import android.support.customtabs.browseractions.BrowserActionsIntent;
+import android.view.Menu;
+import android.widget.LinearLayout;
 
+import org.chromium.chrome.browser.contextmenu.ContextMenuParams;
 import org.chromium.chrome.browser.init.AsyncInitializationActivity;
 import org.chromium.chrome.browser.init.ChromeBrowserInitializer;
 import org.chromium.chrome.browser.util.IntentUtils;
+import org.chromium.content_public.common.Referrer;
+import org.chromium.ui.base.WindowAndroid.OnCloseContextMenuListener;
 
 import java.util.ArrayList;
 
 /**
- * A transparent {@link AsyncInitializationActivity} that displays the browser action context menu.
+ * A transparent {@link AsyncInitializationActivity} that displays the Browser Actions context menu.
  */
 public class BrowserActionActivity extends AsyncInitializationActivity {
     private int mType;
     private Uri mUri;
     private String mCreatorPackageName;
     private ArrayList<BrowserActionItem> mActions = new ArrayList<>();
+    private OnCloseContextMenuListener mCloseContextMenuListener;
 
     @Override
     protected void setContentView() {
@@ -37,16 +43,35 @@ public class BrowserActionActivity extends AsyncInitializationActivity {
         if (mUri == null || mCreatorPackageName == null) {
             super.onDestroy();
         }
-        openContextMenu();
+        LinearLayout layout = new LinearLayout(this);

[Ted C, 2017/04/19 19:58:41]

why do you need this?

Does getWindow().getDecorView() not work?  Is setContentView required by
Activity (we always do it, but I don't know what happens if you don't).

If we do need this, why does this need to be a LinearLayout?  Would just a
vanilla View work?

[ltian, 2017/04/21 05:10:54]

Just try that if we don't have setContentView, the old version (using Android
Context Menu API one) context menu is not displayed. And also pass
getWindow().getDecorView() to openContextMenu does not work even we call
setContentView. But creating a simple View is definitely enough.

+        setContentView(layout);
+        openContextMenu(layout);
     }
 
     /**
      * Opens a Browser Actions context menu based on the parsed data.
      */
-    public void openContextMenu() {
+    public void openContextMenu(LinearLayout layout) {

[Ted C, 2017/04/19 19:58:41]

Unless you need some specific behavior of LinearLayout, this should be passed in
as either a View or ViewGroup as needed.

[ltian, 2017/04/21 05:10:53]

Thanks for pointing out, I think passing in a View is a better option.

+        ContextMenuParams params = getContextMenuParams();
+        BrowserActionsContextMenuHelper helper =
+                new BrowserActionsContextMenuHelper(this, params, mActions);
+        helper.displayBrowserActionsMenu(layout);
         return;
     }
 
+    /**
+     * Populates a {@link ContextMenuParams}.
+     * All the urls of ContextMenuParams are populated by the url from the Browser Actions Intent.
+     * imageWasFetchedLoFi} and canSavemedia are set false.
+     * Referrer policy is set as {@link Referrer.REFERRER_POLICY_DEFAULT}.
+     * @return The ContextMenuParams used to construct context menu.
+     */
+    private ContextMenuParams getContextMenuParams() {
+        Referrer referrer = new Referrer(mUri.toString(), Referrer.REFERRER_POLICY_DEFAULT);

[Ted C, 2017/04/19 19:58:41]

The referrer isn't normally the URL that you long clicked on but the URL of the
current page.  I think this should either be null or use the logic in
IntentHandler.getReferrerUrlIncludingExtraHeaders.

@mariakhomenko, does this sound reasonable?

[Maria, 2017/04/19 21:16:01]

I think this should be an android-app:// URL of the application initiating the
context menu.

You can construct the android-app:// URL using the package name for the
application. Seems like that's something you should be able to know here?
(mCreatorPackageName?)

[ltian, 2017/04/21 05:10:53]

Done.

+        return new ContextMenuParams(mType, mUri.toString(), mUri.toString(), mUri.toString(),
+                mUri.toString(), mUri.toString(), mUri.toString(), false, referrer, false);
+    }
+
     private void beginLoadingLibrary() {
         ChromeBrowserInitializer.getInstance(getApplicationContext()).handlePreNativeStartup(this);
     }
@@ -101,7 +126,24 @@ public class BrowserActionActivity extends AsyncInitializationActivity {
     /**
      * Callback when Browser Actions menu dialog is shown.
      */
-    private void onMenuShow() {
+    public void onMenuShow() {
         beginLoadingLibrary();
     }
+
+    /**
+     * Set the {@link OnCloseContextMenuListener} to clean up populator.
+     * @param listener The listener to handle the clean up.
+     */
+    public void setOnCloseContextMenuListener(OnCloseContextMenuListener listener) {

[Ted C, 2017/04/19 19:58:41]

I don't think the helper should be setting this on the activity.  I think we
should hold onto the helper reference and call onContextMenuClosed.

[ltian, 2017/04/21 05:10:54]

Done.

+        mCloseContextMenuListener = listener;
+    }
+
+    @Override
+    public void onContextMenuClosed(Menu menu) {
+        super.onContextMenuClosed(menu);
+        if (mCloseContextMenuListener != null) {
+            mCloseContextMenuListener.onContextMenuClosed();
+        }
+        super.finish();
+    }
 }