bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits

bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits

Follow-up to https://codereview.chromium.org/2730183003:
* The types in IDLTypes.h are traits types with no member functions, so
  using CORE_EXPORT does not have any effect and we can drop it.
* Export the entire NativeValueTraits specializations in
  NativeValueTraitsImpl.h instead of only their nativeValue() member
  functions.
  In addition to avoiding some repetition in the code, doing so also solves
  some linkage issues on Windows with debug component builds, where targets
  such as 'webkit_unit_tests' or 'blink_media_unittests' would fail to
  reference the aforementioned specializations and lead to errors like this:

    FAILED: media_blink_unittests.exe media_blink_unittests.exe.pdb
    E:/b/depot_tools/python276_bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64 False link.exe /nologo /OUT:./media_blink_unittests.exe /PDB:./media_blink_unittests.exe.pdb @./media_blink_unittests.exe.rsp
    V8RecordTest.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: static class WTF::Vector<struct std::pair<class WTF::String,int>,0,class WTF::PartitionAllocator> __cdecl blink::NativeValueTraits<struct blink::IDLRecord<struct blink::IDLString,struct blink::IDLLong> >::nativeValue(class v8::Isolate *,class v8::Local<class v8::Value>,class blink::ExceptionState &)" (__imp_?nativeValue@?$NativeValueTraits@U?$IDLRecord@UIDLString@blink@@UIDLLong@2@@blink@@@blink@@SA?AV?$Vector@U?$pair@VString@WTF@@H@std@@$0A@VPartitionAllocator@WTF@@@WTF@@PEAVIsolate@v8@@V?$Local@VValue@v8@@@6@AEAVExceptionState@2@@Z) referenced in function "void __cdecl blink::RecordTestV8Internal::setStringLongRecordMethod(class v8::FunctionCallbackInfo<class v8::Value> const &)" (?setStringLongRecordMethod@RecordTestV8Internal@blink@@YAXAEBV?$FunctionCallbackInfo@VValue@v8@@@v8@@@Z)

  The exception to the rule here is IDLSequence: since it needs template
  parameters of its own, it is not a full specialization of
  NativeValueTraitsBase, and at least win_clang complains when it is
  exported:

    E:\b\c\b\win_clang\src\third_party\WebKit\Source\bindings\core\v8\NativeValueTraitsImpl.h(289,8):  error: 'dllimport' attribute ignored [-Werror,-Wignored-attributes]
    struct CORE_EXPORT NativeValueTraits<IDLSequence<T>>

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2742453002
Cr-Commit-Position: refs/heads/master@{#455750}
Committed: https://chromium.googlesource.com/chromium/src/+/5254358bc67bb4f7ef54cc68a69387d5aabd719e

[Raphael Kubo da Costa (rakuco), 2017-03-08 18:17:55]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 18:18:53]

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

[Raphael Kubo da Costa (rakuco), 2017-03-08 18:41:11]

The CQ bit was checked by raphael.kubo.da.costa@intel.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 18:41:21]

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

[Raphael Kubo da Costa (rakuco), 2017-03-08 18:41:47]

Description was changed from

==========
bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits

Follow-up to https://codereview.chromium.org/2730183003:
* The types in IDLTypes.h are traits types with no member functions, so
  using CORE_EXPORT does not have any effect and we can drop it.
* Export the entire NativeValueTraits specializations in
  NativeValueTraitsImpl.h instead of only their nativeValue() member
  functions.
  In addition to avoiding some repetition in the code, doing so also solves
  some linkage issues on Windows with debug component builds, where targets
  such as 'webkit_unit_tests' or 'blink_media_unittests' would fail to
  reference the aforementioned specializations and lead to errors like this:

FAILED: media_blink_unittests.exe media_blink_unittests.exe.pdb
E:/b/depot_tools/python276_bin/python.exe
../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64 False
link.exe /nologo /OUT:./media_blink_unittests.exe
/PDB:./media_blink_unittests.exe.pdb @./media_blink_unittests.exe.rsp
V8RecordTest.obj : error LNK2019: unresolved external symbol
"__declspec(dllimport) public: static class WTF::Vector<struct std::pair<class
WTF::String,int>,0,class WTF::PartitionAllocator> __cdecl
blink::NativeValueTraits<struct blink::IDLRecord<struct blink::IDLString,struct
blink::IDLLong> >::nativeValue(class v8::Isolate *,class v8::Local<class
v8::Value>,class blink::ExceptionState &)"
(__imp_?nativeValue@?$NativeValueTraits@U?$IDLRecord@UIDLString@blink@@UIDLLong@2@@blink@@@blink@@SA?AV?$Vector@U?$pair@VString@WTF@@H@std@@$0A@VPartitionAllocator@WTF@@@WTF@@PEAVIsolate@v8@@V?$Local@VValue@v8@@@6@AEAVExceptionState@2@@Z)
referenced in function "void __cdecl
blink::RecordTestV8Internal::setStringLongRecordMethod(class
v8::FunctionCallbackInfo<class v8::Value> const &)"
(?setStringLongRecordMethod@RecordTestV8Internal@blink@@YAXAEBV?$FunctionCallbackInfo@VValue@v8@@@v8@@@Z)

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

to

==========
bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits

Follow-up to https://codereview.chromium.org/2730183003:
* The types in IDLTypes.h are traits types with no member functions, so
  using CORE_EXPORT does not have any effect and we can drop it.
* Export the entire NativeValueTraits specializations in
  NativeValueTraitsImpl.h instead of only their nativeValue() member
  functions.
  In addition to avoiding some repetition in the code, doing so also solves
  some linkage issues on Windows with debug component builds, where targets
  such as 'webkit_unit_tests' or 'blink_media_unittests' would fail to
  reference the aforementioned specializations and lead to errors like this:

    FAILED: media_blink_unittests.exe media_blink_unittests.exe.pdb
    E:/b/depot_tools/python276_bin/python.exe
../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64 False
link.exe /nologo /OUT:./media_blink_unittests.exe
/PDB:./media_blink_unittests.exe.pdb @./media_blink_unittests.exe.rsp
    V8RecordTest.obj : error LNK2019: unresolved external symbol
"__declspec(dllimport) public: static class WTF::Vector<struct std::pair<class
WTF::String,int>,0,class WTF::PartitionAllocator> __cdecl
blink::NativeValueTraits<struct blink::IDLRecord<struct blink::IDLString,struct
blink::IDLLong> >::nativeValue(class v8::Isolate *,class v8::Local<class
v8::Value>,class blink::ExceptionState &)"
(__imp_?nativeValue@?$NativeValueTraits@U?$IDLRecord@UIDLString@blink@@UIDLLong@2@@blink@@@blink@@SA?AV?$Vector@U?$pair@VString@WTF@@H@std@@$0A@VPartitionAllocator@WTF@@@WTF@@PEAVIsolate@v8@@V?$Local@VValue@v8@@@6@AEAVExceptionState@2@@Z)
referenced in function "void __cdecl
blink::RecordTestV8Internal::setStringLongRecordMethod(class
v8::FunctionCallbackInfo<class v8::Value> const &)"
(?setStringLongRecordMethod@RecordTestV8Internal@blink@@YAXAEBV?$FunctionCallbackInfo@VValue@v8@@@v8@@@Z)

  The exception to the rule here is IDLSequence: since it needs template
  parameters of its own, it is not a full specialization of
  NativeValueTraitsBase, and at least win_clang complains when it is
  exported:

   
E:\b\c\b\win_clang\src\third_party\WebKit\Source\bindings\core\v8\NativeValueTraitsImpl.h(289,8):
 error: 'dllimport' attribute ignored [-Werror,-Wignored-attributes]
    struct CORE_EXPORT NativeValueTraits<IDLSequence<T>>

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

[Raphael Kubo da Costa (rakuco), 2017-03-08 19:25:59]

PTAL. This should fix the Windows issues the bots have reported in
https://codereview.chromium.org/2732093003/. I've also tried this patch together
with that CL on Linux and Windows, and everything built fine.

[haraken, 2017-03-08 19:45:57]

LGTM

[commit-bot: I haz the power, 2017-03-08 20:39:07]

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

[commit-bot: I haz the power, 2017-03-08 20:39:07]

Dry run: This issue passed the CQ dry run.

[Yuki, 2017-03-09 07:55:29]

LGTM with a comment.

https://codereview.chromium.org/2742453002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2742453002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:289: struct
NativeValueTraits<IDLSequence<T>>
I think that you need CORE_TEMPLATE_CLASS_EXPORT here.

Also, IIUC, we should put CORE_TEMPLATE_CLASS_EXPORT for all the template base
classes: NativaValueTraits and NativeValueTraitsBase (including partial
specializations).

Otherwise, we'll hit a similar problem.  Please correct me if I'm wrong.

[Raphael Kubo da Costa (rakuco), 2017-03-09 08:34:50]

https://codereview.chromium.org/2742453002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2742453002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:289: struct
NativeValueTraits<IDLSequence<T>>
On 2017/03/09 07:55:29, Yuki(slow_til_mar08) wrote:
> I think that you need CORE_TEMPLATE_CLASS_EXPORT here.
> 
> Also, IIUC, we should put CORE_TEMPLATE_CLASS_EXPORT for all the template base
> classes: NativaValueTraits and NativeValueTraitsBase (including partial
> specializations).
> 
> Otherwise, we'll hit a similar problem.  Please correct me if I'm wrong.

This was all quite confusing and I'm still not 100% sure, but what I understood
is this: CORE_TEMPLATE_CLASS_EXPORT only seems to be used together with
CORE_TEMPLATE_EXPORT, and they both follow a pattern where certain
specializations are declared either in the header or the .cpp file. For example:

  template <typename T>
  class CORE_TEMPLATE_CLASS_EXPORT C {
    ...
  };

  template class CORE_TEMPLATE_EXPORT C<int>;
  template class CORE_TEMPLATE_EXPORT C<char>;

which is not the case here. What we do here looks more similar to e.g.
//base/json/json_value_converter.h:

  template <>
  class BASE_EXPORT BasicValueConverter<bool> : public ValueConverter<bool> {

  vs.

  template <typename FieldType>
  class CustomFieldConverter : public ValueConverter<FieldType> {

additionally, I've tested this with https://codereview.chromium.org/2732093003/
on Windows and it built correctly.

[Yuki, 2017-03-09 08:41:07]

LGTM!

https://codereview.chromium.org/2742453002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h (right):

https://codereview.chromium.org/2742453002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/bindings/core/v8/NativeValueTraitsImpl.h:289: struct
NativeValueTraits<IDLSequence<T>>
On 2017/03/09 08:34:49, Raphael Kubo da Costa (rakuco) wrote:
> On 2017/03/09 07:55:29, Yuki(slow_til_mar08) wrote:
> > I think that you need CORE_TEMPLATE_CLASS_EXPORT here.
> > 
> > Also, IIUC, we should put CORE_TEMPLATE_CLASS_EXPORT for all the template
base
> > classes: NativaValueTraits and NativeValueTraitsBase (including partial
> > specializations).
> > 
> > Otherwise, we'll hit a similar problem.  Please correct me if I'm wrong.
> 
> This was all quite confusing and I'm still not 100% sure, but what I
understood
> is this: CORE_TEMPLATE_CLASS_EXPORT only seems to be used together with
> CORE_TEMPLATE_EXPORT, and they both follow a pattern where certain
> specializations are declared either in the header or the .cpp file. For
example:
> 
>   template <typename T>
>   class CORE_TEMPLATE_CLASS_EXPORT C {
>     ...
>   };
> 
>   template class CORE_TEMPLATE_EXPORT C<int>;
>   template class CORE_TEMPLATE_EXPORT C<char>;
> 
> which is not the case here. What we do here looks more similar to e.g.
> //base/json/json_value_converter.h:
> 
>   template <>
>   class BASE_EXPORT BasicValueConverter<bool> : public ValueConverter<bool> {
> 
>   vs.
> 
>   template <typename FieldType>
>   class CustomFieldConverter : public ValueConverter<FieldType> {
> 
> additionally, I've tested this with
https://codereview.chromium.org/2732093003/
> on Windows and it built correctly.

Good to know.  You should be right.

[Raphael Kubo da Costa (rakuco), 2017-03-09 15:22:36]

The CQ bit was checked by raphael.kubo.da.costa@intel.com

[commit-bot: I haz the power, 2017-03-09 15:22:59]

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

[commit-bot: I haz the power, 2017-03-09 15:26:28]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1489072956656290,
"parent_rev": "222de9fb1677cdabfbf8998c17dd0e394f71a81e", "commit_rev":
"5254358bc67bb4f7ef54cc68a69387d5aabd719e"}

[commit-bot: I haz the power, 2017-03-09 15:27:06]

Description was changed from

==========
bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits

Follow-up to https://codereview.chromium.org/2730183003:
* The types in IDLTypes.h are traits types with no member functions, so
  using CORE_EXPORT does not have any effect and we can drop it.
* Export the entire NativeValueTraits specializations in
  NativeValueTraitsImpl.h instead of only their nativeValue() member
  functions.
  In addition to avoiding some repetition in the code, doing so also solves
  some linkage issues on Windows with debug component builds, where targets
  such as 'webkit_unit_tests' or 'blink_media_unittests' would fail to
  reference the aforementioned specializations and lead to errors like this:

    FAILED: media_blink_unittests.exe media_blink_unittests.exe.pdb
    E:/b/depot_tools/python276_bin/python.exe
../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64 False
link.exe /nologo /OUT:./media_blink_unittests.exe
/PDB:./media_blink_unittests.exe.pdb @./media_blink_unittests.exe.rsp
    V8RecordTest.obj : error LNK2019: unresolved external symbol
"__declspec(dllimport) public: static class WTF::Vector<struct std::pair<class
WTF::String,int>,0,class WTF::PartitionAllocator> __cdecl
blink::NativeValueTraits<struct blink::IDLRecord<struct blink::IDLString,struct
blink::IDLLong> >::nativeValue(class v8::Isolate *,class v8::Local<class
v8::Value>,class blink::ExceptionState &)"
(__imp_?nativeValue@?$NativeValueTraits@U?$IDLRecord@UIDLString@blink@@UIDLLong@2@@blink@@@blink@@SA?AV?$Vector@U?$pair@VString@WTF@@H@std@@$0A@VPartitionAllocator@WTF@@@WTF@@PEAVIsolate@v8@@V?$Local@VValue@v8@@@6@AEAVExceptionState@2@@Z)
referenced in function "void __cdecl
blink::RecordTestV8Internal::setStringLongRecordMethod(class
v8::FunctionCallbackInfo<class v8::Value> const &)"
(?setStringLongRecordMethod@RecordTestV8Internal@blink@@YAXAEBV?$FunctionCallbackInfo@VValue@v8@@@v8@@@Z)

  The exception to the rule here is IDLSequence: since it needs template
  parameters of its own, it is not a full specialization of
  NativeValueTraitsBase, and at least win_clang complains when it is
  exported:

   
E:\b\c\b\win_clang\src\third_party\WebKit\Source\bindings\core\v8\NativeValueTraitsImpl.h(289,8):
 error: 'dllimport' attribute ignored [-Werror,-Wignored-attributes]
    struct CORE_EXPORT NativeValueTraits<IDLSequence<T>>

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org
==========

to

==========
bindings: Adjust usage of CORE_EXPORT in IDLTypes and NativeValueTraits

Follow-up to https://codereview.chromium.org/2730183003:
* The types in IDLTypes.h are traits types with no member functions, so
  using CORE_EXPORT does not have any effect and we can drop it.
* Export the entire NativeValueTraits specializations in
  NativeValueTraitsImpl.h instead of only their nativeValue() member
  functions.
  In addition to avoiding some repetition in the code, doing so also solves
  some linkage issues on Windows with debug component builds, where targets
  such as 'webkit_unit_tests' or 'blink_media_unittests' would fail to
  reference the aforementioned specializations and lead to errors like this:

    FAILED: media_blink_unittests.exe media_blink_unittests.exe.pdb
    E:/b/depot_tools/python276_bin/python.exe
../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x64 False
link.exe /nologo /OUT:./media_blink_unittests.exe
/PDB:./media_blink_unittests.exe.pdb @./media_blink_unittests.exe.rsp
    V8RecordTest.obj : error LNK2019: unresolved external symbol
"__declspec(dllimport) public: static class WTF::Vector<struct std::pair<class
WTF::String,int>,0,class WTF::PartitionAllocator> __cdecl
blink::NativeValueTraits<struct blink::IDLRecord<struct blink::IDLString,struct
blink::IDLLong> >::nativeValue(class v8::Isolate *,class v8::Local<class
v8::Value>,class blink::ExceptionState &)"
(__imp_?nativeValue@?$NativeValueTraits@U?$IDLRecord@UIDLString@blink@@UIDLLong@2@@blink@@@blink@@SA?AV?$Vector@U?$pair@VString@WTF@@H@std@@$0A@VPartitionAllocator@WTF@@@WTF@@PEAVIsolate@v8@@V?$Local@VValue@v8@@@6@AEAVExceptionState@2@@Z)
referenced in function "void __cdecl
blink::RecordTestV8Internal::setStringLongRecordMethod(class
v8::FunctionCallbackInfo<class v8::Value> const &)"
(?setStringLongRecordMethod@RecordTestV8Internal@blink@@YAXAEBV?$FunctionCallbackInfo@VValue@v8@@@v8@@@Z)

  The exception to the rule here is IDLSequence: since it needs template
  parameters of its own, it is not a full specialization of
  NativeValueTraitsBase, and at least win_clang complains when it is
  exported:

   
E:\b\c\b\win_clang\src\third_party\WebKit\Source\bindings\core\v8\NativeValueTraitsImpl.h(289,8):
 error: 'dllimport' attribute ignored [-Werror,-Wignored-attributes]
    struct CORE_EXPORT NativeValueTraits<IDLSequence<T>>

BUG=685754
R=bashi@chromium.org,haraken@chromium.org,yukishiino@chromium.org

Review-Url: https://codereview.chromium.org/2742453002
Cr-Commit-Position: refs/heads/master@{#455750}
Committed:
https://chromium.googlesource.com/chromium/src/+/5254358bc67bb4f7ef54cc68a693...
==========

[commit-bot: I haz the power, 2017-03-09 15:27:07]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/5254358bc67bb4f7ef54cc68a693...