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

Issue 702298 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug



Sign in to add a comment

Window created by CWVUIDelegate should be closable.

Project Member Reported by michaeldo@chromium.org, Mar 16 2017

Issue description

Window created by CWVUIDelegate should be able to be closed. However, the opened_by_dom is currently being removed, so this will be fixed after https://codereview.chromium.org/2755823002/ lands.
 
This can be done by adding |SetCreatedWithOpener(bool flag) { created_with_opener_ = flag;}| method to WebState.
Labels: -Pri-3 ReleaseBlock-Stable M-63 Pri-1
We need to fix this bug. Otherwise ChromeWebView windows will not be able to close themself.

Comment 3 by cma...@chromium.org, Oct 25 2017

Any update on this RBS?
Status: Started (was: Assigned)
I have a CL in flight now to fix this issue:

https://chromium-review.googlesource.com/740561

Comment 5 by cma...@chromium.org, Oct 31 2017

Is this now fixed?
No, the change has not yet landed.
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 3 2017

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

commit 3e61adc36122a98b47ca177532e79b9a24665690
Author: Mike Dougherty <michaeldo@chromium.org>
Date: Fri Nov 03 00:22:32 2017

Add SetHasOpener API to WebState.

Allow CWVWebView to internally set the value of WebState::HasOpener
after the web state has already been created. This prevents
unneccesarily burdening the embedder of ios/web_view with setting this
value.

Bug:  702298 
Change-Id: Ic4122aeaba619a4276aa24529ca3fe1bc619dfad
Reviewed-on: https://chromium-review.googlesource.com/750195
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#513633}
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web/public/test/fakes/test_web_state.h
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web/public/web_state/web_state.h
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web/web_state/web_state_unittest.mm
[modify] https://crrev.com/3e61adc36122a98b47ca177532e79b9a24665690/ios/web_view/internal/cwv_web_view.mm

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-63; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-63 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-63
Project Member

Comment 11 by sheriffbot@chromium.org, Nov 3 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: Less than 28 days to go before AppStore submit on M63
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 4 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb41cff4d83f6c1d7091f5cc1df9100cd9598614

commit bb41cff4d83f6c1d7091f5cc1df9100cd9598614
Author: Mike Dougherty <michaeldo@chromium.org>
Date: Sat Nov 04 00:25:31 2017

Add SetHasOpener API to WebState.

Allow CWVWebView to internally set the value of WebState::HasOpener
after the web state has already been created. This prevents
unneccesarily burdening the embedder of ios/web_view with setting this
value.

Bug:  702298 
Change-Id: Ic4122aeaba619a4276aa24529ca3fe1bc619dfad
Reviewed-on: https://chromium-review.googlesource.com/750195
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Mike Dougherty <michaeldo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#513633}(cherry picked from commit 3e61adc36122a98b47ca177532e79b9a24665690)
Reviewed-on: https://chromium-review.googlesource.com/753865
Reviewed-by: Mike Dougherty <michaeldo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#375}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web/public/test/fakes/test_web_state.h
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web/public/test/fakes/test_web_state.mm
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web/public/web_state/web_state.h
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web/web_state/web_state_impl.h
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web/web_state/web_state_impl.mm
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web/web_state/web_state_unittest.mm
[modify] https://crrev.com/bb41cff4d83f6c1d7091f5cc1df9100cd9598614/ios/web_view/internal/cwv_web_view.mm

Sign in to add a comment