Move the generation and inclusion of RuntimeEnabledFeatures to blink_platform.dll since many pieces…

Move the generation and inclusion of RuntimeEnabledFeatures to blink_platform.dll since many pieces of platform depend upon this.  Modified the gyp build system and core accordingly.

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

[atreat, 2013-10-10 20:18:23]

This depends upon
https://src.chromium.org/viewvc/chrome?view=rev&revision=159354 which is
experiencing issues at the moment.  But I thought I'd upload this in the
meantime when those issues get sorted out.

[abarth-chromium, 2013-10-10 20:23:42]

lgtm

https://codereview.chromium.org/26763006/diff/1/Source/build/scripts.gypi
File Source/build/scripts.gypi (right):

https://codereview.chromium.org/26763006/diff/1/Source/build/scripts.gypi#new...
Source/build/scripts.gypi:45: '../build/scripts/templates/macros.tmpl',
This file should go in Source/build/scripts/scripts.gypi to follow the pattern
we use elsewhere.

Also, these don't need to "../build/scripts/blah.py".  They can just be
"blah.py" once you move scripts.gypi into place.  The paths resolve relative to
the gypi file if the variable ends in the word "_files" (or a few other magic
names).

https://codereview.chromium.org/26763006/diff/1/Source/core/core.gypi
File Source/core/core.gypi (left):

https://codereview.chromium.org/26763006/diff/1/Source/core/core.gypi#oldcode...
Source/core/core.gypi:3651: ],
You should move these too :)

[Stephen Chennney, 2013-10-11 17:52:17]

Still trying to sort out my patch too. Hopefully I'll resolve the problems by
end of day.

[atreat, 2013-10-16 18:52:13]

On 2013/10/11 17:52:17, Stephen Chenney wrote:
> Still trying to sort out my patch too. Hopefully I'll resolve the problems by
> end of day.

Is this ready to go in now that your change went back in?

[Stephen Chennney, 2013-10-16 21:26:00]

On 2013/10/16 18:52:13, atreat wrote:
> On 2013/10/11 17:52:17, Stephen Chenney wrote:
> > Still trying to sort out my patch too. Hopefully I'll resolve the problems
by
> > end of day.
> 
> Is this ready to go in now that your change went back in?

There will be some conflicts now that my change is in, and you should address
Adam's comments.

Also, you will need to add the RuntimeFeature.cpp and .h files explicitly to
blink_platform.gyp's blink_platform target's sources where I have added
FontFamilyNames.

Upload another patch when you got these things sorted out and give it a go
through the try bots.

[Stephen Chennney, 2013-10-16 21:26:12]

https://codereview.chromium.org/26763006/diff/1/Source/bindings/derived_sourc...
File Source/bindings/derived_sources.gyp (right):

https://codereview.chromium.org/26763006/diff/1/Source/bindings/derived_sourc...
Source/bindings/derived_sources.gyp:33: '../build/scripts.gypi',
This might conflict now.

https://codereview.chromium.org/26763006/diff/1/Source/core/core.gyp
File Source/core/core.gyp (right):

https://codereview.chromium.org/26763006/diff/1/Source/core/core.gyp#newcode35
Source/core/core.gyp:35: '../build/scripts.gypi',
This will conflict too.

[atreat, 2013-10-21 16:07:30]

New version that is applied against latest trunk and fixes the comments I think
from abarth.  Please take a look and let me know!

[Stephen Chennney, 2013-10-21 17:05:07]

On 2013/10/21 16:07:30, atreat wrote:
> New version that is applied against latest trunk and fixes the comments I
think
> from abarth.  Please take a look and let me know!

lgtm, 2. And CQ.

[commit-bot: I haz the power, 2013-10-21 17:05:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26763006/12001

[commit-bot: I haz the power, 2013-10-21 21:14:06]

Retried try job too often on win_blink_rel for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_blink_...

[commit-bot: I haz the power, 2013-10-22 14:45:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26763006/12001

[commit-bot: I haz the power, 2013-10-22 15:20:33]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_layout.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[Stephen Chennney, 2013-10-22 23:59:31]

On 2013/10/22 15:20:33, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build on win_layout.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
> Your code is likely broken or HEAD is junk. Please ensure your
> code is not broken then alert the build sheriffs.
> Look at the try server FAQ for more details.

It appears there is some issue with the BLINK_PLATFORM export. I'll take a look
at this tomorrow for you, when I have access to a Windows machine to verify a
fix.

[Stephen Chennney, 2013-10-23 18:54:32]

On 2013/10/22 23:59:31, Stephen Chenney wrote:
> On 2013/10/22 15:20:33, I haz the power (commit-bot) wrote:
> > Sorry for I got bad news for ya.
> > Compile failed with a clobber build on win_layout.
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
> > Your code is likely broken or HEAD is junk. Please ensure your
> > code is not broken then alert the build sheriffs.
> > Look at the try server FAQ for more details.
> 
> It appears there is some issue with the BLINK_PLATFORM export. I'll take a
look
> at this tomorrow for you, when I have access to a Windows machine to verify a
> fix.

OK. The issue is that the RuntimeEnabledFeaturesCustom.cpp file is in core/page
but implements the method declared in platform/RuntimeEnabledFeatures. The
latter is marked as exported from platform, but the compiler is now seeing it
defined in core.

Best fix is to move that additional file into platform. Be sure to add it to the
blink_platform.gypi file.

[Stephen Chennney, 2013-10-23 19:37:42]

On 2013/10/23 18:54:32, Stephen Chenney wrote:
> On 2013/10/22 23:59:31, Stephen Chenney wrote:
> > On 2013/10/22 15:20:33, I haz the power (commit-bot) wrote:
> > > Sorry for I got bad news for ya.
> > > Compile failed with a clobber build on win_layout.
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
> > > Your code is likely broken or HEAD is junk. Please ensure your
> > > code is not broken then alert the build sheriffs.
> > > Look at the try server FAQ for more details.
> > 
> > It appears there is some issue with the BLINK_PLATFORM export. I'll take a
> look
> > at this tomorrow for you, when I have access to a Windows machine to verify
a
> > fix.
> 
> OK. The issue is that the RuntimeEnabledFeaturesCustom.cpp file is in
core/page
> but implements the method declared in platform/RuntimeEnabledFeatures. The
> latter is marked as exported from platform, but the compiler is now seeing it
> defined in core.
> 
> Best fix is to move that additional file into platform. Be sure to add it to
the
> blink_platform.gypi file.

Turns out that this is insufficient, for various reasons. I think we'll need the
sharedWorkerEnabled feature to a specific core/page class because it requires a
bunch of core code to implement yet we cannot declare it in platform (for
obvious reasons, in hindsight).

[Stephen Chennney, 2013-10-23 19:47:42]

On 2013/10/23 19:37:42, Stephen Chenney wrote:
> On 2013/10/23 18:54:32, Stephen Chenney wrote:
> > On 2013/10/22 23:59:31, Stephen Chenney wrote:
> > > On 2013/10/22 15:20:33, I haz the power (commit-bot) wrote:
> > > > Sorry for I got bad news for ya.
> > > > Compile failed with a clobber build on win_layout.
> > > >
> > >
> >
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
> > > > Your code is likely broken or HEAD is junk. Please ensure your
> > > > code is not broken then alert the build sheriffs.
> > > > Look at the try server FAQ for more details.
> > > 
> > > It appears there is some issue with the BLINK_PLATFORM export. I'll take a
> > look
> > > at this tomorrow for you, when I have access to a Windows machine to
verify
> a
> > > fix.
> > 
> > OK. The issue is that the RuntimeEnabledFeaturesCustom.cpp file is in
> core/page
> > but implements the method declared in platform/RuntimeEnabledFeatures. The
> > latter is marked as exported from platform, but the compiler is now seeing
it
> > defined in core.
> > 
> > Best fix is to move that additional file into platform. Be sure to add it to
> the
> > blink_platform.gypi file.
> 
> Turns out that this is insufficient, for various reasons. I think we'll need
the
> sharedWorkerEnabled feature to a specific core/page class because it requires
a
> bunch of core code to implement yet we cannot declare it in platform (for
> obvious reasons, in hindsight).

I propose we have 2 RuntimeEnabledFeatures files, one for each of platform and
core. I really don't see another option at this point. We should only move
feature flags to platform that are actually required in platform. Maybe name the
files RuntimeEnabledPlatformFeaures and RuntimeEnabledCoreFeatures?

[abarth-chromium, 2013-10-24 14:03:59]

On 2013/10/23 19:47:42, Stephen Chenney wrote:
> I propose we have 2 RuntimeEnabledFeatures files, one for each of platform and
> core. I really don't see another option at this point. We should only move
> feature flags to platform that are actually required in platform. Maybe name
the
> files RuntimeEnabledPlatformFeaures and RuntimeEnabledCoreFeatures?

That seems like more complexity than we need.  There's only one line of code in
RuntimeEnabledFeaturesCustom.cpp:

bool RuntimeEnabledFeatures::sharedWorkerEnabled()
{
    return SharedWorkerRepository::isAvailable();
}

This function looks to be called from exactly one place (ShardWorder.idl).  Why
don't we delete RuntimeEnabledFeaturesCustom.cpp and have ShardWorder.idl talk
to SharedWorkerRepository::isAvailable() directly?

[Stephen Chennney, 2013-10-24 14:12:49]

On 2013/10/24 14:03:59, abarth wrote:
> On 2013/10/23 19:47:42, Stephen Chenney wrote:
> > I propose we have 2 RuntimeEnabledFeatures files, one for each of platform
and
> > core. I really don't see another option at this point. We should only move
> > feature flags to platform that are actually required in platform. Maybe name
> the
> > files RuntimeEnabledPlatformFeaures and RuntimeEnabledCoreFeatures?
> 
> That seems like more complexity than we need.  There's only one line of code
in
> RuntimeEnabledFeaturesCustom.cpp:
> 
> bool RuntimeEnabledFeatures::sharedWorkerEnabled()
> {
>     return SharedWorkerRepository::isAvailable();
> }
> 
> This function looks to be called from exactly one place (ShardWorder.idl). 
Why
> don't we delete RuntimeEnabledFeaturesCustom.cpp and have ShardWorder.idl talk
> to SharedWorkerRepository::isAvailable() directly?

That occurred to me also. I'll get that done and put up a new patch after
verifying that Windows works. Primary attribution will still go to Adam T.

[atreat, 2013-10-24 14:24:34]

On 2013/10/24 14:12:49, Stephen Chenney wrote:
> On 2013/10/24 14:03:59, abarth wrote:
> > 
> > This function looks to be called from exactly one place (ShardWorder.idl). 
> Why
> > don't we delete RuntimeEnabledFeaturesCustom.cpp and have ShardWorder.idl
talk
> > to SharedWorkerRepository::isAvailable() directly?
> 
> That occurred to me also. I'll get that done and put up a new patch after
> verifying that Windows works. Primary attribution will still go to Adam T.

Alrighty, thanks!

[abarth-chromium, 2013-10-24 14:41:41]

On 2013/10/24 14:12:49, Stephen Chenney wrote:
> That occurred to me also. I'll get that done and put up a new patch after
> verifying that Windows works. Primary attribution will still go to Adam T.

Looks like there is already a CL in the works to delete
RuntimeEnabledFeaturesCustom.cpp:

https://codereview.chromium.org/40143003/

[Stephen Chennney, 2013-10-24 14:59:57]

On 2013/10/24 14:41:41, abarth wrote:
> On 2013/10/24 14:12:49, Stephen Chenney wrote:
> > That occurred to me also. I'll get that done and put up a new patch after
> > verifying that Windows works. Primary attribution will still go to Adam T.
> 
> Looks like there is already a CL in the works to delete
> RuntimeEnabledFeaturesCustom.cpp:
> 
> https://codereview.chromium.org/40143003/

New plan. Let's wait 'til 40143003 lands and then Adam T can just rebase and
commit his patch.

[commit-bot: I haz the power, 2013-10-25 15:16:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26763006/292001

[commit-bot: I haz the power, 2013-10-25 16:21:20]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_layout.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[atreat, 2013-10-25 17:16:38]

On 2013/10/25 16:21:20, I haz the power (commit-bot) wrote:
> Sorry for I got bad news for ya.
> Compile failed with a clobber build on win_layout.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_layout...
> Your code is likely broken or HEAD is junk. Please ensure your
> code is not broken then alert the build sheriffs.
> Look at the try server FAQ for more details.

Sigh, not sure what is going on.  I rebased and this is still broken.  I looked
and 40143003 was landed.

[Stephen Chennney, 2013-10-25 17:50:54]

It's just that core is trying to rebuild RuntimeEnabledFeatures.

https://codereview.chromium.org/26763006/diff/292001/Source/core/core.gyp
File Source/core/core.gyp (right):

https://codereview.chromium.org/26763006/diff/292001/Source/core/core.gyp#new...
Source/core/core.gyp:292:
'<(SHARED_INTERMEDIATE_DIR)/blink/RuntimeEnabledFeatures.cpp',
Remove this line.

[commit-bot: I haz the power, 2013-10-25 19:06:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/adam.treat@samsung.com/26763006/462001

[commit-bot: I haz the power, 2013-10-25 20:06:07]

Change committed as 160613