Forward-declare blink::mojom::EngagementLevel

Forward-declare blink::mojom::EngagementLevel

Avoid adding an include to Document.h, which is itself included in many
places. By not including this generated file we also avoid having to
publicly depend on the header generation for any other code that
includes Document.h.

BUG=688353, 682986

[Arjan van Leeuwen, 2017-02-03 12:32:21]

arjanl@opera.com changed reviewers:
+ dominickn@chromium.org, sigbjornf@opera.com

[Arjan van Leeuwen, 2017-02-03 12:32:22]

This patch forward-declares an enum class instead of including the (generated)
header in Document.h, avoiding dependency issues related to generating the
header for all customers of Document.h. Looks OK?

[dominickn, 2017-02-03 18:21:08]

On 2017/02/03 12:32:22, Arjan van Leeuwen wrote:
> This patch forward-declares an enum class instead of including the (generated)
> header in Document.h, avoiding dependency issues related to generating the
> header for all customers of Document.h. Looks OK?

TIL - you can forward declare enums and use them like this.

lgtm - thanks! Please add 682986 to the bug list for this one.

[sof, 2017-02-03 18:30:18]

core/ owner lgtm.

(issue 682986 has restrictive labels, possible&appropriate to open it up a bit?)

[dominickn, 2017-02-03 18:34:50]

On 2017/02/03 18:30:18, sof wrote:
> core/ owner lgtm.
> 
> (issue 682986 has restrictive labels, possible&appropriate to open it up a
bit?)

Issue 682986 is for the official build failing because of this problem - the
content is basically identical to issue 688353 but there are internal links so I
might leave it restricted and have issue 688353 as the public variant.

[sof, 2017-02-03 18:36:23]

Description was changed from

==========
Forward-declare blink::mojom::EngagementLevel

Avoid adding an include to Document.h, which is itself included in many
places. By not including this generated file we also avoid having to
publicly depend on the header generation for any other code that
includes Document.h.

BUG=688353
==========

to

==========
Forward-declare blink::mojom::EngagementLevel

Avoid adding an include to Document.h, which is itself included in many
places. By not including this generated file we also avoid having to
publicly depend on the header generation for any other code that
includes Document.h.

BUG=688353,682986
==========

[sof, 2017-02-03 18:36:52]

The CQ bit was checked by sigbjornf@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-03 18:37:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sof, 2017-02-03 18:40:21]

On 2017/02/03 18:34:50, dominickn wrote:
> On 2017/02/03 18:30:18, sof wrote:
> > core/ owner lgtm.
> > 
> > (issue 682986 has restrictive labels, possible&appropriate to open it up a
> bit?)
> 
> Issue 682986 is for the official build failing because of this problem - the
> content is basically identical to issue 688353 but there are internal links so
I
> might leave it restricted and have issue 688353 as the public variant.

thanks for the summary, that makes sense - just wondered about the overlap.

@arjanl: kicked off a CQ dry run.

[commit-bot: I haz the power, 2017-02-03 20:09:15]

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

[commit-bot: I haz the power, 2017-02-03 20:09:16]

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_...)

[tapted, 2017-02-06 03:29:22]

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

[tapted, 2017-02-06 03:29:23]

https://codereview.chromium.org/2677543003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Document.h (right):

https://codereview.chromium.org/2677543003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Document.h:74: enum class EngagementLevel;
This is implicitly `int` -- see "(3)" at
http://en.cppreference.com/w/cpp/language/enum#Scoped_enumerations -- but the
mojo header declares it as int32_t

They're "probably" the same size on the platforms Chrome supports, but I think
this should be 

enum class EngagementLevel : int32_t;

anyway, I'm the current chromium sheriff, and this has become Pri-0. This CL
needs a rebase and fails presubmits now that clang-format sorts headers for you,
so I've made https://codereview.chromium.org/2677003002/ and referenced the CL
here.

[Arjan van Leeuwen, 2017-02-06 08:04:32]

On 2017/02/06 03:29:23, tapted wrote:
>
https://codereview.chromium.org/2677543003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/dom/Document.h (right):
> 
>
https://codereview.chromium.org/2677543003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/dom/Document.h:74: enum class EngagementLevel;
> This is implicitly `int` -- see "(3)" at
> http://en.cppreference.com/w/cpp/language/enum#Scoped_enumerations -- but the
> mojo header declares it as int32_t
> 
> They're "probably" the same size on the platforms Chrome supports, but I think
> this should be 
> 
> enum class EngagementLevel : int32_t;
> 
> anyway, I'm the current chromium sheriff, and this has become Pri-0. This CL
> needs a rebase and fails presubmits now that clang-format sorts headers for
you,
> so I've made https://codereview.chromium.org/2677003002/ and referenced the CL
> here.

Thanks +tapted, that should fix it.