New issue
Advanced search Search tips

Issue 897829 link

Starred by 4 users

Issue metadata

Status: Verified
Owner:
Closed: Nov 1
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocking:
issue 696617
issue 728334
issue 867599



Sign in to add a comment

Count activation consumption attempts by a different same-site frame

Project Member Reported by mustaq@chromium.org, Oct 22

Issue description

User Activation v2 allows a frame A to use the activation of another frame B only when B is a descendant of A.  During the finch trial on M69 Canary/Dev, we encountered a single case ( Issue 867599 ) of a site breakage where this restriction was interacting with the popup blocker.  It looks like a site problem in this particular case but there is a chance other sites behaving similarly (our finch trial was too limited to access the risk).

We need to count in M71 (without UAv2) the success/failure of a LocalFrame attempting to consume the activation of another LocalFrame (i.e. same-site).

This data would be helpful for Issue 728334 too, to justify the need of a new iframe attribute to allow activation visibility in subframes.

 
Pls apply appropriate OSs label.
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Cc: benmason@chromium.org
We already cut Beta RC for release this week. Pls have safe fix landed in trunk and request a merge to M71 ASAP so we can take it in for next week beta. Thank you.
Ok, pls update the bug with canary result and request a merge so we can pick it up for next week beta if fix is safe to merge.

Note: We already cut M71 beta RC for Android and Desktop for this week release on Thursday.
Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-71; 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-71 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-71
This will need a merge to "M71", hence adding "Merge-Request-71" label.

mustag@, pls update bug with canary result tomorrow. If change looks good in canary, I will approve the merge. Thank you.
Looks like CL in C#6 has introduced a top crash on canary version 72.0.3591.0 being tracked in Issue 898772.
Labels: -Merge-Request-71 Merge-TBD
Thank you ajah@.

Remvoing "Merge-Request-71" label and applying "Merge-TBD" label  as we can't take this merge in due to stability regression listed at #10.



Status: Assigned (was: Fixed)
Reverting the change.
Project Member

Comment 13 by bugdroid1@chromium.org, Oct 25

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

commit be3a76ad4fd374c4eb5fa3a1ed11cc00ec6331d8
Author: Mustaq Ahmed <mustaq@chromium.org>
Date: Thu Oct 25 15:09:32 2018

Revert "Added histograms for frames+outcomes of user activation users."

This reverts commit 1316c14aaac985a39d3beee0721d68f6063891a5.

Reason for revert: crbug.com/898772

Original change's description:
> Added histograms for frames+outcomes of user activation users.
> 
> Bug:  897829 
> Change-Id: Id1ab2eb8c4bc76c1de959e8f229a8a3298bcbc78
> TBR: rkaplow@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/1297000
> Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#602299}

TBR=bokan@chromium.org,rkaplow@chromium.org,mustaq@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  897829 
Change-Id: I2c2beabe36be787108f1e5fe437bd20988607fad
Reviewed-on: https://chromium-review.googlesource.com/c/1299198
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602710}
[modify] https://crrev.com/be3a76ad4fd374c4eb5fa3a1ed11cc00ec6331d8/third_party/blink/renderer/core/frame/frame_test.cc
[modify] https://crrev.com/be3a76ad4fd374c4eb5fa3a1ed11cc00ec6331d8/third_party/blink/renderer/core/frame/local_frame.cc
[modify] https://crrev.com/be3a76ad4fd374c4eb5fa3a1ed11cc00ec6331d8/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/be3a76ad4fd374c4eb5fa3a1ed11cc00ec6331d8/tools/metrics/histograms/histograms.xml

Project Member

Comment 14 by bugdroid1@chromium.org, Oct 25

Labels: merge-merged-3591
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/46dc0b4901ec878b92d74b30f22daafa49a60c0d

commit 46dc0b4901ec878b92d74b30f22daafa49a60c0d
Author: Mustaq Ahmed <mustaq@chromium.org>
Date: Thu Oct 25 15:16:12 2018

Revert "Added histograms for frames+outcomes of user activation users."

This reverts commit 1316c14aaac985a39d3beee0721d68f6063891a5.

Reason for revert: crbug.com/898772

Original change's description:
> Added histograms for frames+outcomes of user activation users.
> 
> Bug:  897829 
> Change-Id: Id1ab2eb8c4bc76c1de959e8f229a8a3298bcbc78
> TBR: rkaplow@chromium.org
> Reviewed-on: https://chromium-review.googlesource.com/c/1297000
> Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
> Reviewed-by: David Bokan <bokan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#602299}

TBR=bokan@chromium.org,rkaplow@chromium.org,mustaq@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  897829 
Change-Id: I2c2beabe36be787108f1e5fe437bd20988607fad
Reviewed-on: https://chromium-review.googlesource.com/c/1299198
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#602710}(cherry picked from commit be3a76ad4fd374c4eb5fa3a1ed11cc00ec6331d8)
Reviewed-on: https://chromium-review.googlesource.com/c/1299335
Reviewed-by: Krishna Govind <govind@chromium.org>
Cr-Commit-Position: refs/branch-heads/3591@{#3}
Cr-Branched-From: 9ea5237fddb8c0031d3517ec49c3ae94912bacad-refs/heads/master@{#602531}
[modify] https://crrev.com/46dc0b4901ec878b92d74b30f22daafa49a60c0d/third_party/blink/renderer/core/frame/frame_test.cc
[modify] https://crrev.com/46dc0b4901ec878b92d74b30f22daafa49a60c0d/third_party/blink/renderer/core/frame/local_frame.cc
[modify] https://crrev.com/46dc0b4901ec878b92d74b30f22daafa49a60c0d/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/46dc0b4901ec878b92d74b30f22daafa49a60c0d/tools/metrics/histograms/histograms.xml

We're cutting M71 Beta RC tomorow  @ 1:00 PM PT for release on Wednesday, pls request a merge to M71 once safe fix is ready to merge. Thank you.
Per chat with mustaq@, we will take this merge in for next week beta as change is not landed in trunk yet.
NextAction: 2018-11-01
Pls update bug with canary result tomorrow. Thank you.
Blocking: 696617
The NextAction date has arrived: 2018-11-01
Status: Verified (was: Assigned)
Verified the added counters through chrome://histograms/UserActivation in Windows Canary 72.0.3598.0, working as expected.
Labels: -Merge-TBD Merge-Rejected-71
NextAction: ----
Labels: -Merge-Rejected-71 Merge-Request-71
Correcting the request label, oops.
Project Member

Comment 24 by sheriffbot@chromium.org, Nov 1

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #21. 
Pls merge your change to M71 branch 3578 ASAP so we can pick it up for next beta release. Thank you.
Project Member

Comment 27 by bugdroid1@chromium.org, Nov 2

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f

commit ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f
Author: Mustaq Ahmed <mustaq@google.com>
Date: Fri Nov 02 13:47:37 2018

Added histograms for frames+outcomes of user activation consumers.

Bug:  897829 
Change-Id: I652665d6a0531ac820329cb64ce073c3b517c8b6
TBR: rkaplow@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1299537
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604324}(cherry picked from commit 1011f64b2bab81798342a15508cb7c053e252ded)
Reviewed-on: https://chromium-review.googlesource.com/c/1314610
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#471}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f/third_party/blink/renderer/core/frame/frame_test.cc
[modify] https://crrev.com/ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f/third_party/blink/renderer/core/frame/local_frame.cc
[modify] https://crrev.com/ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f/tools/metrics/histograms/histograms.xml

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f

Commit: ef2b1fd64c3a43fcfd43c2c5947e0fa96afc128f
Author: mustaq@google.com
Commiter: mustaq@chromium.org
Date: 2018-11-02 13:47:37 +0000 UTC

Added histograms for frames+outcomes of user activation consumers.

Bug:  897829 
Change-Id: I652665d6a0531ac820329cb64ce073c3b517c8b6
TBR: rkaplow@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/c/1299537
Commit-Queue: Mustaq Ahmed <mustaq@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#604324}(cherry picked from commit 1011f64b2bab81798342a15508cb7c053e252ded)
Reviewed-on: https://chromium-review.googlesource.com/c/1314610
Reviewed-by: Mustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#471}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment