Componentization: make symbols visible.

Componentization: make symbols visible for blink_web and blink_tests.

- binding templates update is to suppress global constructor warning reported by clang/win.

BUG=358074
TBR=tkent@chromium.org

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

[tasak, 2015-06-26 12:32:34]

tasak@google.com changed reviewers:
+ haraken@chromium.org

[tasak, 2015-06-26 12:32:36]

PTAL?

[haraken, 2015-06-26 12:42:14]

LGTM

https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
Source/bindings/templates/interface.cpp:479: #if COMPILER(CLANG)

Please add a comment.

[tasak, 2015-06-26 12:42:40]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2015-06-26 12:42:55]

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

[commit-bot: I haz the power, 2015-06-26 12:52:48]

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

[commit-bot: I haz the power, 2015-06-26 12:52:50]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/3...)

[Nico, 2015-06-26 17:09:37]

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

[Nico, 2015-06-26 17:09:37]

https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
Source/bindings/templates/interface.cpp:479: #if COMPILER(CLANG)
On 2015/06/26 12:42:14, haraken wrote:
> 
> Please add a comment.

Also please only do this on Windows and only in a component build.

[tasak, 2015-06-29 04:17:34]

tasak@google.com changed reviewers:
+ tkent@chromium.org

[tasak, 2015-06-29 04:17:35]

tkent, would you review public/'s change?

https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
Source/bindings/templates/interface.cpp:479: #if COMPILER(CLANG)
On 2015/06/26 17:09:37, Nico wrote:
> On 2015/06/26 12:42:14, haraken wrote:
> > 
> > Please add a comment.
> 
> Also please only do this on Windows and only in a component build.

Done.

[tasak, 2015-06-29 05:10:16]

On 2015/06/29 04:17:35, tasak wrote:
> tkent, would you review public/'s change?
> 
>
https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
> File Source/bindings/templates/interface.cpp (right):
> 
>
https://codereview.chromium.org/1212123003/diff/40001/Source/bindings/templat...
> Source/bindings/templates/interface.cpp:479: #if COMPILER(CLANG)
> On 2015/06/26 17:09:37, Nico wrote:
> > On 2015/06/26 12:42:14, haraken wrote:
> > > 
> > > Please add a comment.
> > 
> > Also please only do this on Windows and only in a component build.
> 
> Done.

I added TBD=tkent@chromium.org

[tasak, 2015-06-29 05:10:22]

The CQ bit was checked by tasak@google.com

[tasak, 2015-06-29 05:10:22]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1212123003/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-06-29 05:10:33]

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

[Nico, 2015-06-29 05:23:26]

https://codereview.chromium.org/1212123003/diff/60001/Source/bindings/templat...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/1212123003/diff/60001/Source/bindings/templat...
Source/bindings/templates/interface.cpp:479: // Suppress warning: global
constructors, because struct WrapperTypeInfo is trivial
Initializing a global with a dllimported function on Window requires a static
initializer, it doesn't matter that WrapperTypeInfo is trivial.

Maybe "// Some of these member functions are dllimported, and initializing
globals with dllimported functions requires a static initializer. Since this
only happens in a component build, suppress the warning. TODO: Try to come up
with a way that doesn't require suppressing this warning" or something like
that.

[commit-bot: I haz the power, 2015-06-29 05:32:51]

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

[tasak, 2015-06-29 05:48:25]

Thank you for comments.

https://codereview.chromium.org/1212123003/diff/60001/Source/bindings/templat...
File Source/bindings/templates/interface.cpp (right):

https://codereview.chromium.org/1212123003/diff/60001/Source/bindings/templat...
Source/bindings/templates/interface.cpp:479: // Suppress warning: global
constructors, because struct WrapperTypeInfo is trivial
On 2015/06/29 05:23:25, Nico wrote:
> Initializing a global with a dllimported function on Window requires a static
> initializer, it doesn't matter that WrapperTypeInfo is trivial.
> 
> Maybe "// Some of these member functions are dllimported, and initializing
> globals with dllimported functions requires a static initializer. Since this
> only happens in a component build, suppress the warning. TODO: Try to come up
> with a way that doesn't require suppressing this warning" or something like
> that.

I see. I will fix the comments.

[tkent, 2015-07-02 23:25:48]

https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
File Source/core/layout/HitTestCache.h (right):

https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
Source/core/layout/HitTestCache.h:92: unsigned m_updateIndex;
Do class member initializers prevent the componentization?

[tasak, 2015-07-03 04:26:25]

On 2015/07/02 23:25:48, tkent wrote:
>
https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
> File Source/core/layout/HitTestCache.h (right):
> 
>
https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
> Source/core/layout/HitTestCache.h:92: unsigned m_updateIndex;
> Do class member initializers prevent the componentization?

Because this causes compiler failure when using clang/win.
I'm still investigating what triggers the compile failure.
I guess, clang/win cannot correctly handle dll-inteface classes with class
member initialization.

[Nico, 2015-07-03 04:47:31]

On Thu, Jul 2, 2015 at 9:26 PM, <tasak@google.com> wrote:

> On 2015/07/02 23:25:48, tkent wrote:
>
>
>
https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
>
>> File Source/core/layout/HitTestCache.h (right):
>>
>
>
>
>
https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
>
>> Source/core/layout/HitTestCache.h:92: unsigned m_updateIndex;
>> Do class member initializers prevent the componentization?
>>
>
> Because this causes compiler failure when using clang/win.
> I'm still investigating what triggers the compile failure.
> I guess, clang/win cannot correctly handle dll-inteface classes with class
> member initialization.


This sounds like http://code.google.com/p/chromium/issues/detail?id=488634

(In general, when clang-cl works differently than cl, please please file a
bug on us instead of just looking for a workaround. That way, we can fix
the problem and reduce long-term maintenance cost. In this case, we already
know about the problem.)

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[blink-reviews, 2015-07-03 05:13:56]

The issue 488634 is just the same as I found.


2015-07-03 13:47 GMT+09:00 Nico Weber <thakis@chromium.org>:

> On Thu, Jul 2, 2015 at 9:26 PM, <tasak@google.com> wrote:
>
>> On 2015/07/02 23:25:48, tkent wrote:
>>
>>
>>
https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
>>
>>> File Source/core/layout/HitTestCache.h (right):
>>>
>>
>>
>>
>>
https://codereview.chromium.org/1212123003/diff/60001/Source/core/layout/HitT...
>>
>>> Source/core/layout/HitTestCache.h:92: unsigned m_updateIndex;
>>> Do class member initializers prevent the componentization?
>>>
>>
>> Because this causes compiler failure when using clang/win.
>> I'm still investigating what triggers the compile failure.
>> I guess, clang/win cannot correctly handle dll-inteface classes with class
>> member initialization.
>
>
> This sounds like http://code.google.com/p/chromium/issues/detail?id=488634
>
>
> (In general, when clang-cl works differently than cl, please please file a
> bug on us instead of just looking for a workaround. That way, we can fix
> the problem and reduce long-term maintenance cost. In this case, we already
> know about the problem.)
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[tkent, 2015-07-03 05:53:01]

ok, lgtm.