New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 811134 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Feb 2018
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug
Proj-XR
Proj-XR-VR



Sign in to add a comment

Use ModalDialogs and ModalDialogManager for permission dialog

Project Member Reported by asimjour@chromium.org, Feb 11 2018

Issue description

Permission dialogs should use ModalDialogManager and ModalDialogViews.

This will guarantee that permission dialogs will be shown in VR as well.
  + Once it's done, we should remove the suppression flag in VR.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 13 2018

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

commit 28042407186a5000da43bc3eb4cd23ed669f5432
Author: Amirhossein Simjour <asimjour@chromium.org>
Date: Tue Feb 13 14:49:33 2018

PermissionDialog: Separate the dialog from control

This is the first step towards moving PermissionController to
use  ModalDialogViews for Permission controller behind a flag.
This CL separates the PermissionDialogController and
PermissionDialogView.

BUG= 811134 

Change-Id: I72101f3c329aa74bb57de8a38c2365a3dac32c47
Reviewed-on: https://chromium-review.googlesource.com/912699
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Amirhossein Simjour <asimjour@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536356}
[modify] https://crrev.com/28042407186a5000da43bc3eb4cd23ed669f5432/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
[add] https://crrev.com/28042407186a5000da43bc3eb4cd23ed669f5432/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogView.java
[modify] https://crrev.com/28042407186a5000da43bc3eb4cd23ed669f5432/chrome/android/java_sources.gni
[modify] https://crrev.com/28042407186a5000da43bc3eb4cd23ed669f5432/chrome/android/javatests/src/org/chromium/chrome/browser/permissions/PermissionTestRule.java

Project Member

Comment 2 by bugdroid1@chromium.org, Feb 20 2018

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

commit 97b4154f1753a5b4412d6280b6202377e6bf886c
Author: Amirhossein Simjour <asimjour@chromium.org>
Date: Tue Feb 20 03:49:59 2018

Permission Dialog based on ModalDialogView

PermissionDialogController uses PermissionAppModalDialogView if
the flag is enabled.
Since the goal is to remove the current implementation based on
AlertDialogs, I didn't introduce common interface between
PermissionAppModalDialogView and PermissionDialogView.

BUG= 811134 

Change-Id: Ia3f5e072eccf4b25800ae430b8af10d263344409
Reviewed-on: https://chromium-review.googlesource.com/916551
Commit-Queue: Amirhossein Simjour <asimjour@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Reviewed-by: Becky Zhou <huayinz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537709}
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/download/DownloadLocationDialogBridge.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/jsdialog/JavascriptTabModalDialog.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/modaldialog/ModalDialogManager.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/modaldialog/ModalDialogView.java
[add] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionAppModalDialogView.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/java_sources.gni
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/android/javatests/src/org/chromium/chrome/browser/modaldialog/ModalDialogManagerTest.java
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/browser/about_flags.cc
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/browser/android/chrome_feature_list.cc
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/browser/android/chrome_feature_list.h
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/97b4154f1753a5b4412d6280b6202377e6bf886c/tools/metrics/histograms/enums.xml

Status: Fixed (was: Assigned)
Labels: M-66 Test-TestPlan
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 27 2018

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

commit 10ae6b6d286f7d5abcfa7981b56226465185034b
Author: Amirhossein Simjour <asimjour@chromium.org>
Date: Tue Feb 27 00:16:10 2018

VR - Enable PermissionDialog with ModalDialogView in VR

When we are in VR, we can enabled the ModalDialogView for
PermissionDialogs. This won't change anything for 2D users.
This will also impact the auto reject of the permission requests in
VR. As a result, if the permission is already granted, the website
will be able to use it.
(All of this is behind flag VrBrowsingNativeAndroidUi)

BUG= 811134 ,779126, 738209 

Change-Id: Ic30362acc53c1e7b15a9d9e54457d1c7b3776a2d
Reviewed-on: https://chromium-review.googlesource.com/926882
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539327}
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/android/java/src/org/chromium/chrome/browser/media/remote/RemoteMediaPlayerBridge.java
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/android/java/src/org/chromium/chrome/browser/permissions/PermissionDialogController.java
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/browser/android/tab_web_contents_delegate_android.cc
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/browser/media/android/router/media_router_dialog_controller_android.cc
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/browser/permissions/permission_manager.cc
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/browser/permissions/permission_manager_unittest.cc
[modify] https://crrev.com/10ae6b6d286f7d5abcfa7981b56226465185034b/chrome/browser/permissions/permission_request_manager.cc

Labels: -Test-TestPlan Test-Complete

Sign in to add a comment