Componentization: fix SupplementTracing<0>::~SupplementTracing<0>(void) already defined.

Componentization: fix SupplementTracing<0>::~SupplementTracing<0>(void) already defined.

After making SupplementTracing<false> visible, we will see:

- blink::Supplement<class blink::Page>::Supplement<class blink::Page>(void) already defined.
- blink::SupplementableBase<class blink::Document,0>::provideSupplement(...)  already defined

To solve the above problem,
- add "extern template class WillBeHeapSupplement<Page>" to Page.h.
- add "extern template class WillBeHeapSupplement<Document>" to Document.h

The Supplements are instantiated in Page.cpp and Document.cpp.

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

[tasak, 2015-04-22 03:14:47]

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

[tasak, 2015-04-22 03:14:48]

PTAL?

https://codereview.chromium.org/1060813005/diff/20001/Source/core/core.gypi
File Source/core/core.gypi (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/core/core.gypi#n...
Source/core/core.gypi:2207: 'dom/DocumentSupplementable.h',
Added missing dom/DocumentSupplementable.h.

https://codereview.chromium.org/1060813005/diff/20001/Source/core/dom/Documen...
File Source/core/dom/DocumentSupplementable.h (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/core/dom/Documen...
Source/core/dom/DocumentSupplementable.h:39: extern template class
CORE_TEMPLATE_EXPORT WillBeHeapSupplement<Document>;
To fix the following link errors:
webcore_shared.dll.lib(webcore_shared.dll) : error LNK2005: "public: void
__cdecl blink::SupplementableBase<class
blink::Document,0>::provideSupplement(char const *,class WTF::PassOwnPtr<class
blink::SupplementBase<class blink::Document,0> >)"
(?provideSupplement@?$SupplementableBase@VDocument@blink@@$0A@@blink@@QEAAXPEBDV?$PassOwnPtr@V?$SupplementBase@VDocument@blink@@$0A@@blink@@@WTF@@@Z)
already defined in modules.DeviceLightDispatcher.obj

webcore_shared.dll.lib(webcore_shared.dll) : error LNK2005: "public: class
blink::SupplementBase<class blink::Document,0> * __cdecl
blink::SupplementableBase<class blink::Document,0>::requireSupplement(char const
*)"
(?requireSupplement@?$SupplementableBase@VDocument@blink@@$0A@@blink@@QEAAPEAV?$SupplementBase@VDocument@blink@@$0A@@2@PEBD@Z)
already defined in modules.DeviceLightDispatcher.obj

we need this "extern template class".

https://codereview.chromium.org/1060813005/diff/20001/Source/core/page/PageSu...
File Source/core/page/PageSupplementable.h (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/core/page/PageSu...
Source/core/page/PageSupplementable.h:15: extern template class
CORE_TEMPLATE_EXPORT HeapSupplement<Page>;
Since both HeapSupplement<Page> and Supplement<Page> are used, need to add the
extern templates here (instead of 'extern template class CORE_TEMPLATE_EXPORT
WillBeHeapSupplement<Page>').

https://codereview.chromium.org/1060813005/diff/20001/Source/core/page/PageSu...
Source/core/page/PageSupplementable.h:16: extern template class
CORE_TEMPLATE_EXPORT Supplement<Page>;
To fix the following link errors:
modules.dll.lib(modules.dll) : error LNK2005: "public: __cdecl
blink::Supplement<class blink::Page>::Supplement<class blink::Page>(void)"
(??0?$Supplement@VPage@blink@@@blink@@QEAA@XZ) already defined in
blink_web.StorageQuotaClientImpl.obj

modules.dll.lib(modules.dll) : error LNK2005: "public: virtual __cdecl
blink::Supplement<class blink::Page>::~Supplement<class blink::Page>(void)"
(??1?$Supplement@VPage@blink@@@blink@@UEAA@XZ) already defined in
blink_web.DatabaseClientImpl.obj

we should make core (not modules) to own Supplement<class blink::Page> (and also
HeapSupplement<class blink::Page>).
So we should explicitly say "extern template class __declspec(dllimport)
Supplement<Page>" for modules / web.
For core, "extern template class Supplement<Page>". Page.cpp instantiates the
class, i.e. template class Supplement<Page>.

https://codereview.chromium.org/1060813005/diff/20001/Source/modules/vibratio...
File Source/modules/vibration/NavigatorVibration.h (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/modules/vibratio...
Source/modules/vibration/NavigatorVibration.h:25: #include "platform/Timer.h"
Supplementable.h includes platform/heap/Handle.h and PageSupplementable.h
includes Supplementable.h.
However, need to explicitly add '#include "platform/heap/Handle.h"'?

https://codereview.chromium.org/1060813005/diff/20001/Source/modules/webdatab...
File Source/modules/webdatabase/DatabaseClient.h (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/modules/webdatab...
Source/modules/webdatabase/DatabaseClient.h:34: #include
"core/page/PageSupplementable.h"
Need this before using WillBeHeapSupplement<Page> to avoid instantiating
HeapSupplement<Page> in modules.

https://codereview.chromium.org/1060813005/diff/20001/Source/platform/Supplem...
File Source/platform/Supplementable.cpp (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/platform/Supplem...
Source/platform/Supplementable.cpp:14: //
blink::SupplementTracing<0>::~SupplementTracing<0>(void)"
To compile Supplementable.h, need this cpp file.
Without this cpp file, MSVC does not compile Supplementable.h with "class
__declspec(dllexport) SupplementTracing<false>".

https://codereview.chromium.org/1060813005/diff/20001/Source/platform/Supplem...
File Source/platform/Supplementable.h (right):

https://codereview.chromium.org/1060813005/diff/20001/Source/platform/Supplem...
Source/platform/Supplementable.h:119: class GC_PLUGIN_IGNORE("crbug.com/476419")
PLATFORM_EXPORT SupplementTracing<false> {
To fix SupplementTracing<0>::~SupplementTracing<0>(void) already defined,
blink_platform.dll should own SupplementTracing<0> instance.
So need to make SupplementTracing<0> PLATFORM_EXPORT.

[haraken, 2015-04-22 05:26:53]

Just help me understand: Why do we need the extern declaration only for
PageSupplementable and DocumentSupplementable? (Why don't we need the extern
declaration for other Supplementable classes?)

https://codereview.chromium.org/1060813005/diff/40001/Source/core/dom/Documen...
File Source/core/dom/DocumentSupplementable.h (right):

https://codereview.chromium.org/1060813005/diff/40001/Source/core/dom/Documen...
Source/core/dom/DocumentSupplementable.h:41: using DocumentSupplement =
WillBeHeapSupplement<Document>;

Not related to your CL, we want to remove these two usings, since other
supplementable classes don't have the usings. We can just use
WillBeHeapSupplement<Document> in the call sites.

If we remove these usings, the extern declaration could go to Document.h. Then
we can remove this file.

https://codereview.chromium.org/1060813005/diff/40001/Source/core/page/PageSu...
File Source/core/page/PageSupplementable.h (right):

https://codereview.chromium.org/1060813005/diff/40001/Source/core/page/PageSu...
Source/core/page/PageSupplementable.h:16: extern template class
CORE_TEMPLATE_EXPORT Supplement<Page>;

extern template class CORE_TEMPLATE_EXPORT WillBeHeapSupplement<Page>;

https://codereview.chromium.org/1060813005/diff/40001/Source/core/page/PageSu...
Source/core/page/PageSupplementable.h:16: extern template class
CORE_TEMPLATE_EXPORT Supplement<Page>;

BTW, can we add this extern declaration to Page.h, instead of adding a new file?

[tasak, 2015-04-22 06:59:37]

Thank you for review.

> Why do we need the extern declaration only for
PageSupplementable and DocumentSupplementable?

I'm not sure. I saw DocumentSupplementable.h and I guessed that some guy wants
not to add '#include "Document.h"' to header files.

So removed PageSupplementable.h and DocumentSupplementable.h.

https://codereview.chromium.org/1060813005/diff/40001/Source/core/dom/Documen...
File Source/core/dom/DocumentSupplementable.h (right):

https://codereview.chromium.org/1060813005/diff/40001/Source/core/dom/Documen...
Source/core/dom/DocumentSupplementable.h:41: using DocumentSupplement =
WillBeHeapSupplement<Document>;
On 2015/04/22 05:26:53, haraken wrote:
> 
> Not related to your CL, we want to remove these two usings, since other
> supplementable classes don't have the usings. We can just use
> WillBeHeapSupplement<Document> in the call sites.
> 
> If we remove these usings, the extern declaration could go to Document.h. Then
> we can remove this file.

Removed DocumentSupplementable.h.

https://codereview.chromium.org/1060813005/diff/40001/Source/core/page/PageSu...
File Source/core/page/PageSupplementable.h (right):

https://codereview.chromium.org/1060813005/diff/40001/Source/core/page/PageSu...
Source/core/page/PageSupplementable.h:16: extern template class
CORE_TEMPLATE_EXPORT Supplement<Page>;
On 2015/04/22 05:26:53, haraken wrote:
> 
> BTW, can we add this extern declaration to Page.h, instead of adding a new
file?

Done.

https://codereview.chromium.org/1060813005/diff/40001/Source/core/page/PageSu...
Source/core/page/PageSupplementable.h:16: extern template class
CORE_TEMPLATE_EXPORT Supplement<Page>;
On 2015/04/22 05:26:53, haraken wrote:
> 
> extern template class CORE_TEMPLATE_EXPORT WillBeHeapSupplement<Page>;

Done.

[haraken, 2015-04-22 09:19:44]

https://codereview.chromium.org/1060813005/diff/80001/Source/core/page/Page.cpp
File Source/core/page/Page.cpp (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/core/page/Page.c...
Source/core/page/Page.cpp:607: template class Supplement<Page>;

Can we use:

  template class WillBeHeapSupplement<Page>;

?

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/encrypte...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/encrypte...
Source/modules/encryptedmedia/MediaKeysController.h:16: class Page;

Don't we need to include Page.h? (You're including Page.h in classes that derive
HeapSupplement<Page>.)

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:36: class Page;

Ditto.

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/storage/...
File Source/modules/storage/StorageNamespaceController.h (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/storage/...
Source/modules/storage/StorageNamespaceController.h:15: class Page;

Ditto.

[tasak, 2015-04-22 10:22:21]

https://codereview.chromium.org/1060813005/diff/80001/Source/core/page/Page.cpp
File Source/core/page/Page.cpp (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/core/page/Page.c...
Source/core/page/Page.cpp:607: template class Supplement<Page>;
On 2015/04/22 09:19:43, haraken wrote:
> 
> Can we use:
> 
>   template class WillBeHeapSupplement<Page>;
> 
> ?

Sorry. I missed. We should use template class WillBeHeapSupplement<Page>.

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/encrypte...
File Source/modules/encryptedmedia/MediaKeysController.h (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/encrypte...
Source/modules/encryptedmedia/MediaKeysController.h:16: class Page;
On 2015/04/22 09:19:43, haraken wrote:
> 
> Don't we need to include Page.h? (You're including Page.h in classes that
derive
> HeapSupplement<Page>.)

Removed class Page;

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/speech/S...
File Source/modules/speech/SpeechRecognitionController.h (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/speech/S...
Source/modules/speech/SpeechRecognitionController.h:36: class Page;
On 2015/04/22 09:19:44, haraken wrote:
> 
> Ditto.

Removed.

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/storage/...
File Source/modules/storage/StorageNamespaceController.h (right):

https://codereview.chromium.org/1060813005/diff/80001/Source/modules/storage/...
Source/modules/storage/StorageNamespaceController.h:15: class Page;
On 2015/04/22 09:19:44, haraken wrote:
> 
> Ditto.

Removed

[haraken, 2015-04-22 10:41:55]

LGTM

You'll need to remove DocumentSupplementable.h from core.gypi.

[tasak, 2015-04-22 11:04:43]

Thank you for review.

On 2015/04/22 10:41:55, haraken wrote:
> LGTM
> 
> You'll need to remove DocumentSupplementable.h from core.gypi.

We don't need to remove, because no one added DocumentSupplementable.h to
core.gypi.
(I did in this CL, but I reverted the change)

[tasak, 2015-04-22 11:04:49]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2015-04-22 11:04:55]

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

[commit-bot: I haz the power, 2015-04-22 11:43:57]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=194208