This happens because infobars have a WebContents scoped owner (the InfoBarService), but there's no equivalent for modals.

+dfalcantara for an idle thought: WDYT about creating a DialogService component for Android which controls all AlertDialog type things? That would work similarly to the InfoBarService and handle navigation, etc. correctly. The main difference is that only one dialog would ever be shown at a time.

I think that'd be fine.  I wouldn't call it a DialogService, though—"Service" has a very specific meaning on Android.  DialogManager, maybe.

Okay. Perhaps the way forward here is to refactor the existing PermissionDialogController to make it a more general DialogManager, which has a queue of DialogDelegates. Each of these would have a native side object; the native side DialogManager would be a WebContentsObserver/WebContentsUserData.

There's a bunch of tricky work around theming dialogs and setting the behaviour when they're dismissed / accepted / canceled. That could be a bit sticky.

Do you picture this thing being a manager of dialogs, in general?  It'd be kind of weird if a promo dialog (or some other native Android-only dialog) ended up beneath some modal javascript dialog.  They should probably also be queued.

Also: what do you mean about theming dialogs?  I'm trying to unify promo dialogs for snowflake for M59; I was kind of hoping to push for a general dialog unification later on.

General dialog unification (GDU) sounds great to me. By "theming" I meant that permission dialogs have a particular icon, but other dialogs don't have icons, or have different structure (e.g. the add to homescreen dialog). We'd have to design the interface such that we could support dialogs with different structures (possibly by allowing an AlertDialog object to be provided by the client; the Manager would just queue things and show them one by one).

Ideally modal javascript dialogs would also be included.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fd7dbe308d04ad7132bae51d86aceb33885b4ab

commit 3fd7dbe308d04ad7132bae51d86aceb33885b4ab
Author: timloh <timloh@chromium.org>
Date: Mon May 29 05:59:15 2017

Hide modal permission prompts on Android upon tab navigation/destruction

This patch makes modal permission prompts on Android disappear upon
tab navigation or destruction. We make the PermissionDialogDelegate on
the C++ side a WebContentsObserver to detect these events, and notify
the Java side so the UI can be removed.

BUG= 699851 

Review-Url: https://codereview.chromium.org/2899973002
Cr-Commit-Position: refs/heads/master@{#475297}

[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/android/BUILD.gn
[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogDelegate.java
[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/android/java_sources.gni
[add] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/android/javatests/src/org/chromium/chrome/browser/permissions/PermissionNavigationTest.java
[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/android/javatests/src/org/chromium/chrome/browser/permissions/PermissionTestCaseBase.java
[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/browser/permissions/permission_dialog_delegate.cc
[modify] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/chrome/browser/permissions/permission_dialog_delegate.h
[add] https://crrev.com/3fd7dbe308d04ad7132bae51d86aceb33885b4ab/content/test/data/android/permission_navigation.html

Test failing in arm64 official test bot, need to fix the test for this change?

I 1085.376s run_tests_on_device(00b5128271721c43)  detected failure in org.chromium.chrome.browser.permissions.PermissionNavigationTest#testNavigationDismissesModalPermissionPrompt. raw output:
I 1085.376s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: numtests=1
I 1085.376s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: stream=
I 1085.376s run_tests_on_device(00b5128271721c43)    org.chromium.chrome.browser.permissions.PermissionNavigationTest:
I 1085.376s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: id=InstrumentationTestRunner
I 1085.377s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: test=testNavigationDismissesModalPermissionPrompt
I 1085.377s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: class=org.chromium.chrome.browser.permissions.PermissionNavigationTest
I 1085.377s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: current=1
I 1085.377s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS_CODE: 1
I 1085.377s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: numtests=1
I 1085.377s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: stream=
I 1085.377s run_tests_on_device(00b5128271721c43)    Failure in testNavigationDismissesModalPermissionPrompt:
I 1085.377s run_tests_on_device(00b5128271721c43)    junit.framework.AssertionFailedError: Dialog not shown
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:79)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:117)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:138)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.chrome.browser.permissions.PermissionNavigationTest.testNavigationDismissesModalPermissionPrompt(PermissionNavigationTest.java:42)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:382)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
I 1085.377s run_tests_on_device(00b5128271721c43)    	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1879)
I 1085.377s run_tests_on_device(00b5128271721c43)    
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: id=InstrumentationTestRunner
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: test=testNavigationDismissesModalPermissionPrompt
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: class=org.chromium.chrome.browser.permissions.PermissionNavigationTest
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: stack=junit.framework.AssertionFailedError: Dialog not shown
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollInstrumentationThread(CriteriaHelper.java:79)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:117)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.content.browser.test.util.CriteriaHelper.pollUiThread(CriteriaHelper.java:138)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.chrome.browser.permissions.PermissionNavigationTest.testNavigationDismissesModalPermissionPrompt(PermissionNavigationTest.java:42)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.test.InstrumentationTestCase.runMethod(InstrumentationTestCase.java:214)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.test.InstrumentationTestCase.runTest(InstrumentationTestCase.java:199)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.test.ActivityInstrumentationTestCase2.runTest(ActivityInstrumentationTestCase2.java:192)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.chrome.test.ChromeActivityTestCaseBase.runTest(ChromeActivityTestCaseBase.java:382)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.base.test.BaseTestResult.runParameterized(BaseTestResult.java:161)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at org.chromium.base.test.BaseTestResult.run(BaseTestResult.java:124)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:191)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.test.AndroidTestRunner.runTest(AndroidTestRunner.java:176)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.test.InstrumentationTestRunner.onStart(InstrumentationTestRunner.java:555)
I 1085.378s run_tests_on_device(00b5128271721c43)    	at android.app.Instrumentation$InstrumentationThread.run(Instrumentation.java:1879)
I 1085.378s run_tests_on_device(00b5128271721c43)    
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS: current=1
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_STATUS_CODE: -2
I 1085.378s run_tests_on_device(00b5128271721c43)    INSTRUMENTATION_RESULT: stream=
I 1085.378s run_tests_on_device(00b5128271721c43)    Test results for ChromeInstrumentationTestRunner=.F
I 1085.379s run_tests_on_device(00b5128271721c43)    Time: 4.347
I 1085.379s run_tests_on_device(00b5128271721c43)    
I 1085.379s run_tests_on_device(00b5128271721c43)    FAILURES!!!
I 1085.379s run_tests_on_device(00b5128271721c43)    Tests run: 1,  Failures: 1,  Errors: 0


Logs:  https://uberchromegw.corp.google.com/i/official.android/builders/test-official-arm_64/builds/1567/steps/Test%3A%20ChromePublicTest/logs/stdio

Marking RBS since causing test to fail on official bot https://uberchromegw.corp.google.com/i/official.android/builders/test-official-arm/builds/1791/steps/Test%3A%20ChromePublicTest/logs/stdio

Pls apply appropriate OSs label. Thank you.

I'm not sure what the official bot does differently to make this fail. Looking at the test, I'd guess that the test page isn't properly loaded so requestGeolocationPermission() ends up not doing anything.

Looks like test is flaky on the official arm64 bot:

passed 61.0.3163.30: https://uberchromegw.corp.google.com/i/official.android/builders/test-official-arm_64/builds/1738/steps/Test%3A%20ChromePublicTest/logs/stdio

failed 61.0.3163.33: https://uberchromegw.corp.google.com/i/official.android/builders/test-official-arm_64/builds/1740/steps/Test%3A%20ChromePublicTest/logs/stdio

passed 61.0.3163.34: https://uberchromegw.corp.google.com/i/official.android/builders/test-official-arm_64/builds/1743/steps/Test%3A%20ChromePublicTest/logs/stdio

Passed 61.0.3163.37:
https://uberchromegw.corp.google.com/i/official.android/builders/test-official-arm_64/builds/1750/steps/Test%3A%20ChromePublicTest/logs/stdio

Removing RBS but please get this fixed ASAP or disable the test.

I'll disable the test for now...

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f88e0a451377ef3bb7639cf312f81399a3fd2595

commit f88e0a451377ef3bb7639cf312f81399a3fd2595
Author: Timothy Loh <timloh@chromium.org>
Date: Tue Aug 08 05:45:32 2017

Disable PermissionNavigationTest which flakes on official bots

PermissionNavigationTest.testNavigationDismissesModalPermissionPrompt is
flaky on official android bots. This patch disables it as I haven't been
able to work out what's causing it to flake.

Bug:  699851 
Change-Id: I1421f3233acab1d59199a68acc98c3da4c35809b
Reviewed-on: https://chromium-review.googlesource.com/605069
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Timothy Loh <timloh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492542}
[modify] https://crrev.com/f88e0a451377ef3bb7639cf312f81399a3fd2595/chrome/android/javatests/src/org/chromium/chrome/browser/permissions/PermissionNavigationTest.java

Technically it was fixed by #7, although the test is disabled now.