Measure the usage of cross-origin modal dialogs.

Measure the usage of cross-origin modal dialogs.

BUG=621336
R=ojan@chromium.org,japhet@chromium.org

Committed: https://crrev.com/825a75cd8469c4dc1ce291e51d96c3dec57e9ecf
Cr-Commit-Position: refs/heads/master@{#401046}

[Bin Lu, 2016-06-20 03:31:58]

Hi Ojan and Nate,

I took some time on this, but am not sure if this is the right way to do it
since this is my first chrome CL.

I tried the IDL way to have a MeasureCrossOrigin attribute as suggested by Ojan,
but didn't figure out how to do the measure with a condition like "it's
cross-origin".

Looking forward to your comments.

Thanks,
Bin

[ojan, 2016-06-20 14:04:45]

ojan@chromium.org changed reviewers:
+ avi@chromium.org

[ojan, 2016-06-20 14:04:45]

The CQ bit was checked by ojan@chromium.org

[ojan, 2016-06-20 14:04:45]

lgtm

[commit-bot: I haz the power, 2016-06-20 14:05:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2079183002/1

[commit-bot: I haz the power, 2016-06-20 14:13:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-20 14:13:32]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Bin Lu, 2016-06-21 04:38:19]

binlu@google.com changed reviewers:
+ dcheng@chromium.org, rkaplow@chromium.org
- avi@chromium.org

[Bin Lu, 2016-06-21 04:38:19]

+rkaplow@ and dcheng@ for the OWNERS review. Thanks.

[dcheng, 2016-06-21 11:20:02]

LGTM with nits

https://codereview.chromium.org/2079183002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp (right):

https://codereview.chromium.org/2079183002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp:768: if
(frame()->isCrossOrigin()) {
Nit: no { } around a single-line conditional body

[rkaplow, 2016-06-21 14:40:06]

lgtm

[Bin Lu, 2016-06-21 15:02:25]

Thanks all for the review!

Since this is my first patch, could someone help request try jobs be run for me?


On 2016/06/21 11:20:02, dcheng wrote:
> LGTM with nits
> 
>
https://codereview.chromium.org/2079183002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp (right):
> 
>
https://codereview.chromium.org/2079183002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp:768: if
> (frame()->isCrossOrigin()) {
> Nit: no { } around a single-line conditional body

Done. Thanks.

[rkaplow, 2016-06-21 15:05:09]

lgtm

Oh, hey bin, didn't notice it was you :)

You can actually just check off the 'commit' button - then it will run a bunch
of the trybots and will submit if they are ok. Since this is a simple change
that should be sufficient.

[Bin Lu, 2016-06-21 15:07:24]

The CQ bit was checked by binlu@google.com

[Bin Lu, 2016-06-21 15:07:24]

The patchset sent to the CQ was uploaded after l-g-t-m from ojan@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2079183002/#ps60001
(title: "Remove { } around a single-line conditional body.")

[Bin Lu, 2016-06-21 15:07:24]

The CQ bit was unchecked by binlu@google.com

[Bin Lu, 2016-06-21 15:08:15]

The CQ bit was checked by binlu@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-06-21 15:08:28]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2079183002/60001

[Bin Lu, 2016-06-21 15:22:32]

Thanks Robert for the info! This is my first Chrome cl :)
I've checked off the commit button, and will see what'll happen.



On 2016/06/21 15:05:09, rkaplow wrote:
> lgtm
> 
> Oh, hey bin, didn't notice it was you :)
> 
> You can actually just check off the 'commit' button - then it will run a bunch
> of the trybots and will submit if they are ok. Since this is a simple change
> that should be sufficient.

[commit-bot: I haz the power, 2016-06-21 17:58:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-21 17:58:02]

Dry run: This issue passed the CQ dry run.

[Bin Lu, 2016-06-21 18:01:48]

The CQ bit was checked by binlu@google.com

[commit-bot: I haz the power, 2016-06-21 18:02:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2079183002/60001

[commit-bot: I haz the power, 2016-06-21 18:08:38]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-06-21 18:16:43]

Description was changed from

==========
Measure the usage of cross-origin modal dialogs.

BUG=621336

R=ojan@chromium.org,japhet@chromium.org
==========

to

==========
Measure the usage of cross-origin modal dialogs.

BUG=621336

R=ojan@chromium.org,japhet@chromium.org

Committed: https://crrev.com/825a75cd8469c4dc1ce291e51d96c3dec57e9ecf
Cr-Commit-Position: refs/heads/master@{#401046}
==========

[commit-bot: I haz the power, 2016-06-21 18:16:46]

Patchset 4 (id:??) landed as
https://crrev.com/825a75cd8469c4dc1ce291e51d96c3dec57e9ecf
Cr-Commit-Position: refs/heads/master@{#401046}

[Bin Lu, 2016-06-28 18:44:17]

Patchset #5 (id:80001) has been deleted