Make UseCounter take a LocaFrame instead of any Frame

Make UseCounter take a LocaFrame instead of any Frame

It doesn't make sense for a UseCounter to be triggered by a RemoteFrame.
Update all the callers to provide a LocalFrame (or in a couple cases
where we can't be sure to have one, skip reporting usage).

In a future patch I'd like to start depending on the fact that we know
the correct LocalFrame for a UseCounter invocation.

BUG=716565
Review-Url: https://codereview.chromium.org/2877893002
Cr-Commit-Position: refs/heads/master@{#475268}
Committed: https://chromium.googlesource.com/chromium/src/+/64819bf38f1754e6677f2be17335a2b8f8103060

[Rick Byers, 2017-05-12 01:35:33]

The CQ bit was checked by rbyers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 01:36:32]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 02:27:39]

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

[commit-bot: I haz the power, 2017-05-12 02:27:39]

Dry run: Try jobs failed on following builders:
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Rick Byers, 2017-05-12 15:31:27]

The CQ bit was checked by rbyers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 15:35:59]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 15:58:00]

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

[commit-bot: I haz the power, 2017-05-12 15:58:01]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Rick Byers, 2017-05-12 16:49:26]

The CQ bit was checked by rbyers@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-12 16:50:19]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-12 19:25:58]

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

[commit-bot: I haz the power, 2017-05-12 19:25:59]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Rick Byers, 2017-05-12 22:00:19]

rbyers@chromium.org changed reviewers:
+ japhet@chromium.org

[Rick Byers, 2017-05-12 22:00:20]

Hey Nate,
PTAL.  This is mostly OOPIF-related and updates some of (I believe) your loading
UseCounters.

/cc andypaicu@ for FrameTree.cpp.  Any concerns about this reducing your metrics
coverage at all? It wasn't entirely clear to me if this would miss any useful
cases for you (or if you still needed this counter).

/cc dtapuska@ for TouchEvent.cpp.  I don't think this will miss any useful
cases, but if you'd prefer to keep them all then I can update
Event::preventDefault to have [CallWith=ScriptState] and get the current
ExecutionContext instead.

/cc clamy@ for MixedContentWarning.cpp.  Changes are all structural, so metrics
should be reported in all the same cases as before.

[dcheng, 2017-05-12 22:15:19]

some drive-by thoughts

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/TouchEvent.cpp (right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/TouchEvent.cpp:263: if (view() &&
view()->IsLocalDOMWindow() && view()->GetFrame()) {
This makes me a little sad, but unfortunately, I see no reason why a TouchEvent
object can't be synthesized while specifying a cross origin view object. =(

I wonder why we even support that, since events can't be dispatched across
origin boundaries anyway.

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

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/Frame.cpp:178: if (IsLocalFrame() &&
target_frame != this && sandboxed &&
I think this can actually only be called on a local frame. We should probably
move this function.

(May be easier to add a DCHECK(IsLocalFrame() and move in a followup)

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/FrameTree.cpp (right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FrameTree.cpp:60:
UseCounter::kCrossOriginMainFrameNulledNameAccessed);
I think we might want to keep this: can we cheat by using Page::GetUseCounter()
directly?

(I'm actually surprised at how few direct references there are to GetUseCounter)

[haraken, 2017-05-15 03:40:44]

haraken@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2017-05-15 03:40:45]

LGTM

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
(right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),

This will work as expected because CurrentExecutionContext returns nullptr for a
remote frame and UseCounter::Count ignores the nullptr execution context.

However, I'd prefer using an explicit isLocalFrame check to make the logic
explicit.

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:335:
CurrentExecutionContext(info.GetIsolate()),

Ditto.

[dcheng, 2017-05-15 04:04:01]

dcheng@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2017-05-15 04:04:02]

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
(right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),
On 2017/05/15 03:40:45, haraken wrote:
> 
> This will work as expected because CurrentExecutionContext returns nullptr for
a
> remote frame and UseCounter::Count ignores the nullptr execution context.
> 
> However, I'd prefer using an explicit isLocalFrame check to make the logic
> explicit.

I'm not sure I follow: I don't think CurrentExecutionContext() can ever
correspond to a context that belongs to a remote frame (since a remote frame's
context can't be entered).

[haraken, 2017-05-15 04:45:45]

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
(right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),
On 2017/05/15 04:04:02, dcheng (in AEST) wrote:
> On 2017/05/15 03:40:45, haraken wrote:
> > 
> > This will work as expected because CurrentExecutionContext returns nullptr
for
> a
> > remote frame and UseCounter::Count ignores the nullptr execution context.
> > 
> > However, I'd prefer using an explicit isLocalFrame check to make the logic
> > explicit.
> 
> I'm not sure I follow: I don't think CurrentExecutionContext() can ever
> correspond to a context that belongs to a remote frame (since a remote frame's
> context can't be entered).

CurrentExecutionContext() returns nullptr for a remote frame. UseCounter::Count
ignores it. As a result, this line is no-op for a remote frame.

I'm just suggesting making the logic more explicit using the IsLocalFrame check.

[dcheng, 2017-05-15 04:57:30]

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
(right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),
On 2017/05/15 04:45:45, haraken wrote:
> On 2017/05/15 04:04:02, dcheng (in AEST) wrote:
> > On 2017/05/15 03:40:45, haraken wrote:
> > > 
> > > This will work as expected because CurrentExecutionContext returns nullptr
> for
> > a
> > > remote frame and UseCounter::Count ignores the nullptr execution context.
> > > 
> > > However, I'd prefer using an explicit isLocalFrame check to make the logic
> > > explicit.
> > 
> > I'm not sure I follow: I don't think CurrentExecutionContext() can ever
> > correspond to a context that belongs to a remote frame (since a remote
frame's
> > context can't be entered).
> 
> CurrentExecutionContext() returns nullptr for a remote frame.
UseCounter::Count
> ignores it. As a result, this line is no-op for a remote frame.
> 
> I'm just suggesting making the logic more explicit using the IsLocalFrame
check.

Right, but CurrentExecutionContext() gets it from
v8::Isolate::GetCurrentContext(), right? It's impossible for v8 to enter a
v8::Context that does not correspond to a LocalFrame.

Thus, even though the bindings code internally looks like it needs to handle the
RemoteFrame case, it's actually impossible. (I agree this is confusing)

[haraken, 2017-05-15 05:00:30]

On 2017/05/15 04:57:30, dcheng (in AEST) wrote:
>
https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
> (right):
> 
>
https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
> third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
> UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),
> On 2017/05/15 04:45:45, haraken wrote:
> > On 2017/05/15 04:04:02, dcheng (in AEST) wrote:
> > > On 2017/05/15 03:40:45, haraken wrote:
> > > > 
> > > > This will work as expected because CurrentExecutionContext returns
nullptr
> > for
> > > a
> > > > remote frame and UseCounter::Count ignores the nullptr execution
context.
> > > > 
> > > > However, I'd prefer using an explicit isLocalFrame check to make the
logic
> > > > explicit.
> > > 
> > > I'm not sure I follow: I don't think CurrentExecutionContext() can ever
> > > correspond to a context that belongs to a remote frame (since a remote
> frame's
> > > context can't be entered).
> > 
> > CurrentExecutionContext() returns nullptr for a remote frame.
> UseCounter::Count
> > ignores it. As a result, this line is no-op for a remote frame.
> > 
> > I'm just suggesting making the logic more explicit using the IsLocalFrame
> check.
> 
> Right, but CurrentExecutionContext() gets it from
> v8::Isolate::GetCurrentContext(), right? It's impossible for v8 to enter a
> v8::Context that does not correspond to a LocalFrame.
> 
> Thus, even though the bindings code internally looks like it needs to handle
the
> RemoteFrame case, it's actually impossible. (I agree this is confusing)

So we'll hit this path, right?

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

Or are you saying that it's not possible for a remote frame to call
namedPropertyGetter() in the first place?

[dcheng, 2017-05-15 05:02:32]

On 2017/05/15 05:00:30, haraken wrote:
> On 2017/05/15 04:57:30, dcheng (in AEST) wrote:
> >
>
https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
> > (right):
> > 
> >
>
https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
> > UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),
> > On 2017/05/15 04:45:45, haraken wrote:
> > > On 2017/05/15 04:04:02, dcheng (in AEST) wrote:
> > > > On 2017/05/15 03:40:45, haraken wrote:
> > > > > 
> > > > > This will work as expected because CurrentExecutionContext returns
> nullptr
> > > for
> > > > a
> > > > > remote frame and UseCounter::Count ignores the nullptr execution
> context.
> > > > > 
> > > > > However, I'd prefer using an explicit isLocalFrame check to make the
> logic
> > > > > explicit.
> > > > 
> > > > I'm not sure I follow: I don't think CurrentExecutionContext() can ever
> > > > correspond to a context that belongs to a remote frame (since a remote
> > frame's
> > > > context can't be entered).
> > > 
> > > CurrentExecutionContext() returns nullptr for a remote frame.
> > UseCounter::Count
> > > ignores it. As a result, this line is no-op for a remote frame.
> > > 
> > > I'm just suggesting making the logic more explicit using the IsLocalFrame
> > check.
> > 
> > Right, but CurrentExecutionContext() gets it from
> > v8::Isolate::GetCurrentContext(), right? It's impossible for v8 to enter a
> > v8::Context that does not correspond to a LocalFrame.
> > 
> > Thus, even though the bindings code internally looks like it needs to handle
> the
> > RemoteFrame case, it's actually impossible. (I agree this is confusing)
> 
> So we'll hit this path, right?
> 
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...
> 
> Or are you saying that it's not possible for a remote frame to call
> namedPropertyGetter() in the first place?

The argument is a v8::Local<v8::Context>. However, remote frames don't use
v8::Context now that we use lightweight global proxy objects for them; thus, it
is not possible to hit a remote frame in this code path. Sorry it is confusing,
and I think I am not explaining it well...

[haraken, 2017-05-15 05:03:30]

On 2017/05/15 05:02:32, dcheng (in AEST) wrote:
> On 2017/05/15 05:00:30, haraken wrote:
> > On 2017/05/15 04:57:30, dcheng (in AEST) wrote:
> > >
> >
>
https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/bindings/core/v8/custom/V8WindowCustom.cpp:321:
> > > UseCounter::Count(CurrentExecutionContext(info.GetIsolate()),
> > > On 2017/05/15 04:45:45, haraken wrote:
> > > > On 2017/05/15 04:04:02, dcheng (in AEST) wrote:
> > > > > On 2017/05/15 03:40:45, haraken wrote:
> > > > > > 
> > > > > > This will work as expected because CurrentExecutionContext returns
> > nullptr
> > > > for
> > > > > a
> > > > > > remote frame and UseCounter::Count ignores the nullptr execution
> > context.
> > > > > > 
> > > > > > However, I'd prefer using an explicit isLocalFrame check to make the
> > logic
> > > > > > explicit.
> > > > > 
> > > > > I'm not sure I follow: I don't think CurrentExecutionContext() can
ever
> > > > > correspond to a context that belongs to a remote frame (since a remote
> > > frame's
> > > > > context can't be entered).
> > > > 
> > > > CurrentExecutionContext() returns nullptr for a remote frame.
> > > UseCounter::Count
> > > > ignores it. As a result, this line is no-op for a remote frame.
> > > > 
> > > > I'm just suggesting making the logic more explicit using the
IsLocalFrame
> > > check.
> > > 
> > > Right, but CurrentExecutionContext() gets it from
> > > v8::Isolate::GetCurrentContext(), right? It's impossible for v8 to enter a
> > > v8::Context that does not correspond to a LocalFrame.
> > > 
> > > Thus, even though the bindings code internally looks like it needs to
handle
> > the
> > > RemoteFrame case, it's actually impossible. (I agree this is confusing)
> > 
> > So we'll hit this path, right?
> > 
> >
>
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...
> > 
> > Or are you saying that it's not possible for a remote frame to call
> > namedPropertyGetter() in the first place?
> 
> The argument is a v8::Local<v8::Context>. However, remote frames don't use
> v8::Context now that we use lightweight global proxy objects for them; thus,
it
> is not possible to hit a remote frame in this code path. Sorry it is
confusing,
> and I think I am not explaining it well...

Ah, makes sense. Thanks!

[andypaicu, 2017-05-15 07:57:07]

On 2017/05/12 at 22:00:20, rbyers wrote:
> Hey Nate,
> PTAL.  This is mostly OOPIF-related and updates some of (I believe) your
loading UseCounters.
> 
> /cc andypaicu@ for FrameTree.cpp.  Any concerns about this reducing your
metrics coverage at all? It wasn't entirely clear to me if this would miss any
useful cases for you (or if you still needed this counter).
> 
> /cc dtapuska@ for TouchEvent.cpp.  I don't think this will miss any useful
cases, but if you'd prefer to keep them all then I can update
Event::preventDefault to have [CallWith=ScriptState] and get the current
ExecutionContext instead.
> 
> /cc clamy@ for MixedContentWarning.cpp.  Changes are all structural, so
metrics should be reported in all the same cases as before.

I don't really have any concerns as I'm probably going to take that counter down
soon anyway.

[Rick Byers, 2017-05-25 15:32:14]

The CQ bit was checked by rbyers@chromium.org to run a CQ dry run

[Rick Byers, 2017-05-25 15:32:31]

Thanks for the feedback and sorry for taking so long to reply.  dcheng@ can you
please take another look?

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/events/TouchEvent.cpp (right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/events/TouchEvent.cpp:263: if (view() &&
view()->IsLocalDOMWindow() && view()->GetFrame()) {
On 2017/05/12 22:15:18, dcheng wrote:
> This makes me a little sad, but unfortunately, I see no reason why a
TouchEvent
> object can't be synthesized while specifying a cross origin view object. =(
> 
> I wonder why we even support that, since events can't be dispatched across
> origin boundaries anyway.

Yeah I can't imagine a use case, but it would also be weird to add a special
case to the event creation system to disallow doing this.  The idea with event
creation is that script should be able to plumb whatever values it wants into
the events, even when the UA itself would never do that (eg. mouse events with
bogus/inconsistent co-ordinates).  So I think it's consistent with the larger
design there.

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

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/Frame.cpp:178: if (IsLocalFrame() &&
target_frame != this && sandboxed &&
On 2017/05/12 22:15:18, dcheng wrote:
> I think this can actually only be called on a local frame. We should probably
> move this function.
> 
> (May be easier to add a DCHECK(IsLocalFrame() and move in a followup)

Good catch, and looks like it wasn't too hard to move so I just did it in this
CL - hope that's OK.  Code is simpler now (though harder to diff, sorry).

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/page/FrameTree.cpp (right):

https://codereview.chromium.org/2877893002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/page/FrameTree.cpp:60:
UseCounter::kCrossOriginMainFrameNulledNameAccessed);
On 2017/05/12 22:15:18, dcheng wrote:
> I think we might want to keep this: can we cheat by using
Page::GetUseCounter()
> directly?
> 
> (I'm actually surprised at how few direct references there are to
GetUseCounter)

Unfortunately no, in order to migrate UseCounter handling to the browser process
I really need to fix all this "cheating" and associate a real LocalFrame with
each call to Count.  Fundamentally there's always some local frame responsible
for triggering usage, and we might want to treat the usage differently depending
on which frame triggered it.

That said, it's not that big of a deal at the moment to cheat by picking _some_
LocalFrame in the page, but I figured we wouldn't want to search the whole frame
tree to find one.  So in practice I figured it was probably fine to cheat by
picking either the current or top frame here when one was local.  That should
handle most of the cases I think.  

This especially didn't seem worth worrying about too much when the TODO implied
the measurement would likely go away soon.

[commit-bot: I haz the power, 2017-05-25 15:32:40]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-25 18:30:50]

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

[commit-bot: I haz the power, 2017-05-25 18:30:51]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dcheng, 2017-05-27 08:16:21]

lgtm

https://codereview.chromium.org/2877893002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/2877893002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:1263: void
UseCounter::Count(Feature feature, const LocalFrame* sourceFrame) {
Nit: source_frame

[Rick Byers, 2017-05-28 18:19:16]

https://codereview.chromium.org/2877893002/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/frame/UseCounter.cpp (right):

https://codereview.chromium.org/2877893002/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/frame/UseCounter.cpp:1263: void
UseCounter::Count(Feature feature, const LocalFrame* sourceFrame) {
On 2017/05/27 08:16:21, dcheng wrote:
> Nit: source_frame

Done, thanks

[Rick Byers, 2017-05-28 18:19:23]

The CQ bit was checked by rbyers@chromium.org

[Rick Byers, 2017-05-28 18:19:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2877893002/#ps140001
(title: "Fix style")

[commit-bot: I haz the power, 2017-05-28 18:19:34]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-28 18:31:22]

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

[commit-bot: I haz the power, 2017-05-28 18:31:23]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Rick Byers, 2017-05-28 23:04:36]

The CQ bit was checked by rbyers@chromium.org

[Rick Byers, 2017-05-28 23:04:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2877893002/#ps160001
(title: "Fix compile")

[commit-bot: I haz the power, 2017-05-28 23:04:47]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-29 01:08:40]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1496012676818140,
"parent_rev": "8cb8d28de8f2c439a50307ddc142c533c5b79e2c", "commit_rev":
"64819bf38f1754e6677f2be17335a2b8f8103060"}

[commit-bot: I haz the power, 2017-05-29 01:08:56]

Description was changed from

==========
Make UseCounter take a LocaFrame instead of any Frame

It doesn't make sense for a UseCounter to be triggered by a RemoteFrame.
Update all the callers to provide a LocalFrame (or in a couple cases
where we can't be sure to have one, skip reporting usage).

In a future patch I'd like to start depending on the fact that we know
the correct LocalFrame for a UseCounter invocation.

BUG=716565
==========

to

==========
Make UseCounter take a LocaFrame instead of any Frame

It doesn't make sense for a UseCounter to be triggered by a RemoteFrame.
Update all the callers to provide a LocalFrame (or in a couple cases
where we can't be sure to have one, skip reporting usage).

In a future patch I'd like to start depending on the fact that we know
the correct LocalFrame for a UseCounter invocation.

BUG=716565

Review-Url: https://codereview.chromium.org/2877893002
Cr-Commit-Position: refs/heads/master@{#475268}
Committed:
https://chromium.googlesource.com/chromium/src/+/64819bf38f1754e6677f2be17335...
==========

[commit-bot: I haz the power, 2017-05-29 01:08:57]

Committed patchset #9 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/64819bf38f1754e6677f2be17335...