Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2807743002:
Fix a linker error when building content_unittests (Closed)
DescriptionFix a linker error when building content_unittests
When building content_unittests for Cast on Android with mojo_media_host
as "browser", linker complains media::MojoRendererService::Create() is
undefined. Add //media/mojo/services as a dependency when
enable_mojo_media is true to resolve the error.
BUG=Internal b/37086171
Review-Url: https://codereview.chromium.org/2807743002
Cr-Commit-Position: refs/heads/master@{#463310}
Committed: https://chromium.googlesource.com/chromium/src/+/9ebda5bc1beadd3e5509b1d838f71b8ef3d8892e
Patch Set 1 #
Messages
Total messages: 17 (6 generated)
Description was changed from ========== Fix a linker error when building content_unittests When building content_unittests for Cast on Android with mojo_media_host as "browser", linker complains media::MojoRendererService::Create() is undefined. Add //media/mojo/services as a dependency when enable_mojo_media is true to resolve the error. BUG=Internal b/37086171 ========== to ========== Fix a linker error when building content_unittests When building content_unittests for Cast on Android with mojo_media_host as "browser", linker complains media::MojoRendererService::Create() is undefined. Add //media/mojo/services as a dependency when enable_mojo_media is true to resolve the error. BUG=Internal b/37086171 ==========
tsunghung@chromium.org changed reviewers: + alokp@chromium.org, halliwell@chromium.org, phajdan.jr@chromium.org, sanfin@chromium.org
On 2017/04/07 19:10:01, AndyWu wrote: lgtm
alokp@chromium.org changed reviewers: + xhwang@chromium.org
Thanks! OOC, which test fails to build because of this?
On 2017/04/08 04:35:50, xhwang_slow wrote: > Thanks! > > OOC, which test fails to build because of this? content/test:content_unittests fails to build. media::MojoRendererService::Create() is invoked here: https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_... However, it only declare "//media/mojo/interfaces", not "//media/mojo/services", as a dependency. https://cs.chromium.org/chromium/src/content/browser/BUILD.gn?l=93
LGTM
The CQ bit was checked by tsunghung@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/04/08 17:41:51, AndyWu wrote: > On 2017/04/08 04:35:50, xhwang_slow wrote: > > Thanks! > > > > OOC, which test fails to build because of this? > > content/test:content_unittests fails to build. > > media::MojoRendererService::Create() is invoked here: > https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_... > However, it only declare "//media/mojo/interfaces", not "//media/mojo/services", > as a dependency. > https://cs.chromium.org/chromium/src/content/browser/BUILD.gn?l=93 I am a bit confused about this fix. The code referred above is production code, in render_frame_host_impl.cc. I am not sure why changing content/test/BUILD.gn would fix that. Also, it seems we already have this deps for render_frame_host_impl.cc: https://cs.chromium.org/chromium/src/content/browser/BUILD.gn?rcl=de94e722f95...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1491841812079710, "parent_rev":
"71ffa0d2e073d09d8098b0794f7f382655aabedc", "commit_rev":
"9ebda5bc1beadd3e5509b1d838f71b8ef3d8892e"}
Message was sent while issue was closed.
Description was changed from ========== Fix a linker error when building content_unittests When building content_unittests for Cast on Android with mojo_media_host as "browser", linker complains media::MojoRendererService::Create() is undefined. Add //media/mojo/services as a dependency when enable_mojo_media is true to resolve the error. BUG=Internal b/37086171 ========== to ========== Fix a linker error when building content_unittests When building content_unittests for Cast on Android with mojo_media_host as "browser", linker complains media::MojoRendererService::Create() is undefined. Add //media/mojo/services as a dependency when enable_mojo_media is true to resolve the error. BUG=Internal b/37086171 Review-Url: https://codereview.chromium.org/2807743002 Cr-Commit-Position: refs/heads/master@{#463310} Committed: https://chromium.googlesource.com/chromium/src/+/9ebda5bc1beadd3e5509b1d838f7... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/9ebda5bc1beadd3e5509b1d838f7...
Message was sent while issue was closed.
On 2017/04/10 16:38:47, xhwang_slow wrote: > On 2017/04/08 17:41:51, AndyWu wrote: > > On 2017/04/08 04:35:50, xhwang_slow wrote: > > > Thanks! > > > > > > OOC, which test fails to build because of this? > > > > content/test:content_unittests fails to build. > > > > media::MojoRendererService::Create() is invoked here: > > > https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_... > > However, it only declare "//media/mojo/interfaces", not > "//media/mojo/services", > > as a dependency. > > https://cs.chromium.org/chromium/src/content/browser/BUILD.gn?l=93 > > I am a bit confused about this fix. > > The code referred above is production code, in render_frame_host_impl.cc. I am > not sure why changing content/test/BUILD.gn would fix that. > > Also, it seems we already have this deps for render_frame_host_impl.cc: > https://cs.chromium.org/chromium/src/content/browser/BUILD.gn?rcl=de94e722f95... You are absolutely right! It was added 6 days ago, and it should be enough to fix our issue found 7 days ago. I will revert this CL. Thanks a lot for your help!
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:1) has been created in https://codereview.chromium.org/2808943002/ by tsunghung@chromium.org. The reason for reverting is: The issue should be fixed already with the CL: https://codereview.chromium.org/2792293002.. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
