|
|
Created:
3 years, 8 months ago by tguilbert Modified:
3 years, 7 months ago Reviewers:
ncarter (slow) CC:
chromium-reviews, nasko+codewatch_chromium.org, mlamouri+watch-content_chromium.org, jam, darin-cc_chromium.org, creis+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionSimplify RFI::CreateMediaPlayer
Since CourierRendererFactory (used to be called AdaptiveRendererFactory)
no longer takes a RendererFactory as a construction parameter, we can
clean up the logic in RFI::CreateMediaPlayer, so that is is simpler to
read.
BUG=663503
Review-Url: https://codereview.chromium.org/2822353003
Cr-Commit-Position: refs/heads/master@{#470129}
Committed: https://chromium.googlesource.com/chromium/src/+/9affbc15342e08bc0162756c9d1ec32fdf492fab
Patch Set 1 #Patch Set 2 : Rebase #
Total comments: 2
Patch Set 3 : SetBaseFactoryType #Patch Set 4 : Fix build errors #Messages
Total messages: 22 (12 generated)
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Simplify RFI::CreateMediaPlayer //TODO(tguilbert) BUG= ========== to ========== Simplify RFI::CreateMediaPlayer Since CourierRendererFactory (used to be called AdaptiveRendererFactory) no longer takes a RendererFactory as a construction parameter, we can clean up the logic in RFI::CreateMediaPlayer, so that is is simpler to read. BUG=663503 ==========
tguilbert@chromium.org changed reviewers: + nick@chromium.org
PTAL! This is the last CL in the series of CLs following the introduction of the RendererFactorySelector. Can you OWNERS review RenderFrameImpl? Thank you! Thomas
https://codereview.chromium.org/2822353003/diff/60001/content/renderer/render... File content/renderer/render_frame_impl.cc (left): https://codereview.chromium.org/2822353003/diff/60001/content/renderer/render... content/renderer/render_frame_impl.cc:2978: factory_selector->SetBaseFactoryType(factory_type); Is it no longer necessary to call SetBaseFactoryType?
https://codereview.chromium.org/2822353003/diff/60001/content/renderer/render... File content/renderer/render_frame_impl.cc (left): https://codereview.chromium.org/2822353003/diff/60001/content/renderer/render... content/renderer/render_frame_impl.cc:2978: factory_selector->SetBaseFactoryType(factory_type); On 2017/05/03 23:25:32, ncarter wrote: > Is it no longer necessary to call SetBaseFactoryType? It is! Thank you for catching this!
lgtm
The CQ bit was checked by tguilbert@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org Link to the patchset: https://codereview.chromium.org/2822353003/#ps100001 (title: "Fix build errors")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by tguilbert@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by tguilbert@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch. Bot data: {"patchset_id": 100001, "attempt_start_ts": 1494272647195480, "parent_rev": "a925ec8c709dcc3768dfeb349bf19c766d6f5c7d", "commit_rev": "9affbc15342e08bc0162756c9d1ec32fdf492fab"}
Message was sent while issue was closed.
Description was changed from ========== Simplify RFI::CreateMediaPlayer Since CourierRendererFactory (used to be called AdaptiveRendererFactory) no longer takes a RendererFactory as a construction parameter, we can clean up the logic in RFI::CreateMediaPlayer, so that is is simpler to read. BUG=663503 ========== to ========== Simplify RFI::CreateMediaPlayer Since CourierRendererFactory (used to be called AdaptiveRendererFactory) no longer takes a RendererFactory as a construction parameter, we can clean up the logic in RFI::CreateMediaPlayer, so that is is simpler to read. BUG=663503 Review-Url: https://codereview.chromium.org/2822353003 Cr-Commit-Position: refs/heads/master@{#470129} Committed: https://chromium.googlesource.com/chromium/src/+/9affbc15342e08bc0162756c9d1e... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:100001) as https://chromium.googlesource.com/chromium/src/+/9affbc15342e08bc0162756c9d1e... |