Support deprecation warnings in shared/service workers

Support deprecation warnings in shared/service workers

This patch makes service worker and shared worker send console message
to its worker console when a deprecation feature is used.
Still these workers do not record usage, as they do not have a notion
corresponds to page visit.

BUG=376039, 451334
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189789

[Kunihiko Sakamoto, 2015-02-05 05:43:39]

Patchset #1 (id:1) has been deleted

[Kunihiko Sakamoto, 2015-02-05 06:11:11]

ksakamoto@chromium.org changed reviewers:
+ kinuko@chromium.org, sigbjornf@opera.com

[Kunihiko Sakamoto, 2015-02-05 06:11:12]

This enables us to give heads-up to developers about deprecation,
although we cannot measure actual usage.

I've tested it manually with https://codereview.chromium.org/892053003/.

[sof, 2015-02-05 08:48:18]

Good thinking, it makes sense to separately support deprecation messages for
shared/service workers.

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
File Source/core/workers/WorkerGlobalScope.cpp (right):

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerGlobalScope.cpp:340: 
Could you add a comment (or an assert, if you prefer) making it clear what kinds
of global scopes are handled here?

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerGlobalScope.cpp:350:
executionContext()->addConsoleMessage(ConsoleMessage::create(DeprecationMessageSource,
WarningMessageLevel, UseCounter::deprecationMessage(feature)));
We know currently from calling context that executionContext() will be valid,
but could you add an assert?

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
File Source/core/workers/WorkerGlobalScope.h (right):

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerGlobalScope.h:163: mutable OwnPtr<BitVector>
m_deprecationWarningBits;
Did you consider factoring UseCounter a bit so as to allow sharing of
representation & recording of measurements? That would also allow inspector to
hook into its support for muting.

[Kunihiko Sakamoto, 2015-02-05 10:32:25]

Patchset #2 (id:40001) has been deleted

[Kunihiko Sakamoto, 2015-02-05 10:35:37]

Thanks for the review.

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
File Source/core/workers/WorkerGlobalScope.cpp (right):

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerGlobalScope.cpp:340: 
On 2015/02/05 08:48:18, sof wrote:
> Could you add a comment (or an assert, if you prefer) making it clear what
kinds
> of global scopes are handled here?

Done.

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerGlobalScope.cpp:350:
executionContext()->addConsoleMessage(ConsoleMessage::create(DeprecationMessageSource,
WarningMessageLevel, UseCounter::deprecationMessage(feature)));
On 2015/02/05 08:48:18, sof wrote:
> We know currently from calling context that executionContext() will be valid,
> but could you add an assert?

Done.

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
File Source/core/workers/WorkerGlobalScope.h (right):

https://codereview.chromium.org/901143002/diff/20001/Source/core/workers/Work...
Source/core/workers/WorkerGlobalScope.h:163: mutable OwnPtr<BitVector>
m_deprecationWarningBits;
On 2015/02/05 08:48:18, sof wrote:
> Did you consider factoring UseCounter a bit so as to allow sharing of
> representation & recording of measurements? That would also allow inspector to
> hook into its support for muting.

Factored it to UseCounter::CountBits class.

[sof, 2015-02-05 20:48:31]

Thanks. For test coverage, could you put together one for both kinds of worker
supported here? Use console.timeline to trigger the deprecations, perhaps.

[kinuko, 2015-02-09 06:10:51]

Thanks for working on this.  I think this lgtm, as sof@ writes it'd be much
nicer to have a test coverage but afaik we don't have a good infra for testing
console output from Workers yet, do we?  Adding such infra would be actually
beneficial for all other worker work (i.e. for SW), but probably it could be
done in a separate patch. (Wdyt, Sigbjorn?)

[Kunihiko Sakamoto, 2015-02-09 07:57:44]

On 2015/02/09 06:10:51, kinuko wrote:
> Thanks for working on this.  I think this lgtm, as sof@ writes it'd be much
> nicer to have a test coverage but afaik we don't have a good infra for testing
> console output from Workers yet, do we?  Adding such infra would be actually
> beneficial for all other worker work (i.e. for SW), but probably it could be
> done in a separate patch. (Wdyt, Sigbjorn?)

Thanks kinuko. Filed crbug.com/456708 for the test infra work.

[sof, 2015-02-09 08:20:17]

On 2015/02/09 07:57:44, Kunihiko Sakamoto wrote:
> On 2015/02/09 06:10:51, kinuko wrote:
> > Thanks for working on this.  I think this lgtm, as sof@ writes it'd be much
> > nicer to have a test coverage but afaik we don't have a good infra for
testing
> > console output from Workers yet, do we?  Adding such infra would be actually
> > beneficial for all other worker work (i.e. for SW), but probably it could be
> > done in a separate patch. (Wdyt, Sigbjorn?)
> 
> Thanks kinuko. Filed crbug.com/456708 for the test infra work.

I thought that was in place (cf. fast/workers/shared-worker-console-log.html),
but its expected output is rather sparse. 

So, yes, addressing that underlying shortcoming separately makes perfect sense;
thanks for filing the bug.

lgtm to proceed.

[Kunihiko Sakamoto, 2015-02-09 08:22:33]

The CQ bit was checked by ksakamoto@chromium.org

[commit-bot: I haz the power, 2015-02-09 08:23:30]

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

[commit-bot: I haz the power, 2015-02-09 10:02:42]

Committed patchset #2 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189789