MediaRecorder: exporting WebMediaRecorderHandler.h and adding dummy WebMediaRecorderHandler.cpp

MediaRecorder: exporting WebMediaRecorderHandler.h and adding dummy WebMediaRecorderHandler.cpp

BUG=262211
Committed: https://crrev.com/4cbaf34ae3c0451429d00f154b035e98a7e18b17
git-svn-id: svn://svn.chromium.org/blink/trunk@202081 bbb929c8-8fbe-4397-9dbb-9b2b20218538

[mcasas, 2015-09-10 20:05:21]

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

[mcasas, 2015-09-10 20:05:21]

thakis@ tiny CL PTAL.

[Nico, 2015-09-10 21:01:39]

I'd be somewhat surprised if this works (and even if it does, it kind of seems
like the wrong fix).

While looking at this shortly, I noticed that WebMediaRecorderHandler.h isn't
listed in gyp/gn files. That won't cause this problem, but it'll let the analyze
step have incorrect results.

I saw that there's no cc file for this. I think the Right Fix is to add a cpp
file and include the .h file there. Then the compiler has a place where the
dllexported functions can go. I'm guessing that currently no cc file in the
target containing the header file includes the header, and so the dllexported
functions never get exported, only imported.

You can repro the problem by doing a component build on your windows box.

[mcasas, 2015-09-10 21:11:30]

On 2015/09/10 21:01:39, Nico (afk Tue-Wed Sep 7-8) wrote:
> I'd be somewhat surprised if this works (and even if it does, it kind of seems
> like the wrong fix).
> 
> While looking at this shortly, I noticed that WebMediaRecorderHandler.h isn't
> listed in gyp/gn files. That won't cause this problem, but it'll let the
analyze
> step have incorrect results.
> 

Ouch you're right. Reverted change and adding my file to BUILD.gn.

> I saw that there's no cc file for this. I think the Right Fix is to add a cpp
> file and include the .h file there. Then the compiler has a place where the
> dllexported functions can go. I'm guessing that currently no cc file in the
> target containing the header file includes the header, and so the dllexported
> functions never get exported, only imported.

Yeah I found those pesky import-export troubles in win
in an unrelated CL and there was something similar during 
early landing of the mentioned class(es).
(https://codereview.chromium.org/1255873002/#msg63).

Will add a dummy .cpp and ping you.

> 
> You can repro the problem by doing a component build on your windows box.

I wish! My Win box is, perhaps unsurprisingly, down when I need it.

[Nico, 2015-09-10 21:49:04]

https://codereview.chromium.org/1337543002/diff/40001/public/BUILD.gn
File public/BUILD.gn (right):

https://codereview.chromium.org/1337543002/diff/40001/public/BUILD.gn#newcode131
public/BUILD.gn:131: "platform/WebMediaRecorder.h",
You probably need to add this to some .gyp file too

[mcasas, 2015-09-10 22:03:50]

https://codereview.chromium.org/1337543002/diff/40001/public/BUILD.gn
File public/BUILD.gn (right):

https://codereview.chromium.org/1337543002/diff/40001/public/BUILD.gn#newcode131
public/BUILD.gn:131: "platform/WebMediaRecorder.h",
On 2015/09/10 21:49:04, Nico (afk Tue-Wed Sep 7-8) wrote:
> You probably need to add this to some .gyp file too

None of the other platform/* seem to need any explicit exporting
in a .gyp(i) file (e.g. WebCompositorAnimationPlayer, 
WebServiceWorkerResponse, WebMediaStream), I think relying 
on Source//platform/blink_platform.gypi for transitive export.

[Nico, 2015-09-10 22:06:58]

On 2015/09/10 22:03:50, mcasas wrote:
> https://codereview.chromium.org/1337543002/diff/40001/public/BUILD.gn
> File public/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1337543002/diff/40001/public/BUILD.gn#newcode131
> public/BUILD.gn:131: "platform/WebMediaRecorder.h",
> On 2015/09/10 21:49:04, Nico (afk Tue-Wed Sep 7-8) wrote:
> > You probably need to add this to some .gyp file too
> 
> None of the other platform/* seem to need any explicit exporting
> in a .gyp(i) file (e.g. WebCompositorAnimationPlayer, 
> WebServiceWorkerResponse, WebMediaStream), I think relying 
> on Source//platform/blink_platform.gypi for transitive export.

The gypi file contains the .cpp file. It's true that this is enough for the .cpp
file. The .h file is only listed in the .gn file though from what I can tell.

[Nico, 2015-09-10 22:11:42]

lgtm after chat discussion (no other .h file is listed in gyp currently either.
we should fix that, but not in this cl.)

[mcasas, 2015-09-10 22:23:52]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2015-09-10 22:24:19]

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

[commit-bot: I haz the power, 2015-09-11 00:15:46]

Committed patchset #3 (id:40001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=202081

[commit-bot: I haz the power, 2015-09-23 12:16:45]

Patchset 3 (id:??) landed as
https://crrev.com/4cbaf34ae3c0451429d00f154b035e98a7e18b17