Update UsageCounter methods to be callable on Frames.

Update UsageCounter methods to be callable on Frames.

In the near future, Window.idl will be implemented as DOMWindow, not
LocalDOMWindow. Some of the custom Window bindings try to call
UsageCounter methods using Document as the reference, but DOMWindows
do not have a Document if they are remote.

BUG=425623
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=185023

[dcheng, 2014-11-08 23:02:50]

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

[dcheng, 2014-11-08 23:02:50]

This is the UseCounter stuff I was referring to in
https://codereview.chromium.org/683013002. This is needed because postMessage()
can be called on remote DOMWindows, so there might be no Document to use a
reference point.

I have mixed feelings about this patch. It'd be nice to reduce the number of
overloads, but UseCounter::count(Document&, Feature) is used in a massive amount
of places.

[haraken, 2014-11-10 02:40:23]

LGTM

On 2014/11/08 23:02:50, dcheng wrote:
> This is the UseCounter stuff I was referring to in
> https://codereview.chromium.org/683013002. This is needed because
postMessage()
> can be called on remote DOMWindows, so there might be no Document to use a
> reference point.
> 
> I have mixed feelings about this patch. It'd be nice to reduce the number of
> overloads, but UseCounter::count(Document&, Feature) is used in a massive
amount
> of places.

Yeah, it's not nice to have UseCounter::count(Document&),
UseCounter::count(Frame*) and UseCounter::count(ExecutionContext&). Would it
mess up the code base if we remove UseCounter::count(Document&, Feature)? If no,
we might want to remove UseCounter::count(Document&, Feature).

[dcheng, 2014-11-10 03:46:34]

On 2014/11/10 at 02:40:23, haraken wrote:
> LGTM
> 
> On 2014/11/08 23:02:50, dcheng wrote:
> > This is the UseCounter stuff I was referring to in
> > https://codereview.chromium.org/683013002. This is needed because
postMessage()
> > can be called on remote DOMWindows, so there might be no Document to use a
> > reference point.
> > 
> > I have mixed feelings about this patch. It'd be nice to reduce the number of
> > overloads, but UseCounter::count(Document&, Feature) is used in a massive
amount
> > of places.
> 
> Yeah, it's not nice to have UseCounter::count(Document&),
UseCounter::count(Frame*) and UseCounter::count(ExecutionContext&). Would it
mess up the code base if we remove UseCounter::count(Document&, Feature)? If no,
we might want to remove UseCounter::count(Document&, Feature).

We could remove UseCounter::count(Document&, Feature), but instead of:
  UseCounter::count(document(), feature)
You would have to write:
  UseCounter::count(document().frame(), feature)

I can make this change in a followup patch (I wanted to see what it looked like
combined into this patch, but there were a lot of callers)

[haraken, 2014-11-10 04:00:02]

On 2014/11/10 03:46:34, dcheng wrote:
> On 2014/11/10 at 02:40:23, haraken wrote:
> > LGTM
> > 
> > On 2014/11/08 23:02:50, dcheng wrote:
> > > This is the UseCounter stuff I was referring to in
> > > https://codereview.chromium.org/683013002. This is needed because
> postMessage()
> > > can be called on remote DOMWindows, so there might be no Document to use a
> > > reference point.
> > > 
> > > I have mixed feelings about this patch. It'd be nice to reduce the number
of
> > > overloads, but UseCounter::count(Document&, Feature) is used in a massive
> amount
> > > of places.
> > 
> > Yeah, it's not nice to have UseCounter::count(Document&),
> UseCounter::count(Frame*) and UseCounter::count(ExecutionContext&). Would it
> mess up the code base if we remove UseCounter::count(Document&, Feature)? If
no,
> we might want to remove UseCounter::count(Document&, Feature).
> 
> We could remove UseCounter::count(Document&, Feature), but instead of:
>   UseCounter::count(document(), feature)
> You would have to write:
>   UseCounter::count(document().frame(), feature)

If we have a lot of places where we have to write document().frame(), it would
make sense to have a helper method in UseCounter.h.

[dcheng, 2014-11-10 04:03:01]

On 2014/11/10 at 04:00:02, haraken wrote:
> On 2014/11/10 03:46:34, dcheng wrote:
> > On 2014/11/10 at 02:40:23, haraken wrote:
> > > LGTM
> > > 
> > > On 2014/11/08 23:02:50, dcheng wrote:
> > > > This is the UseCounter stuff I was referring to in
> > > > https://codereview.chromium.org/683013002. This is needed because
> > postMessage()
> > > > can be called on remote DOMWindows, so there might be no Document to use
a
> > > > reference point.
> > > > 
> > > > I have mixed feelings about this patch. It'd be nice to reduce the
number of
> > > > overloads, but UseCounter::count(Document&, Feature) is used in a
massive
> > amount
> > > > of places.
> > > 
> > > Yeah, it's not nice to have UseCounter::count(Document&),
> > UseCounter::count(Frame*) and UseCounter::count(ExecutionContext&). Would it
> > mess up the code base if we remove UseCounter::count(Document&, Feature)? If
no,
> > we might want to remove UseCounter::count(Document&, Feature).
> > 
> > We could remove UseCounter::count(Document&, Feature), but instead of:
> >   UseCounter::count(document(), feature)
> > You would have to write:
> >   UseCounter::count(document().frame(), feature)
> 
> If we have a lot of places where we have to write document().frame(), it would
make sense to have a helper method in UseCounter.h.

OK, that was my feeling too after trying to proceed down this route initially.
This is used by a lot of subclasses of Node, which only have a Document
reference. This patch has already changed the Document& overloads to just
forward to the Frame versions--these should be functionally equivalent.

[dcheng, 2014-11-10 04:03:07]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-11-10 04:03:35]

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

[commit-bot: I haz the power, 2014-11-10 06:31:10]

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

[commit-bot: I haz the power, 2014-11-10 06:31:11]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/35531)

[dcheng, 2014-11-10 06:31:33]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-11-10 06:31:44]

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

[commit-bot: I haz the power, 2014-11-10 07:12:12]

Committed patchset #2 (id:20001) as 185023