New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 697672 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug



Sign in to add a comment

Label missing when remoting video on Windows

Project Member Reported by sko...@chromium.org, Mar 2 2017

Issue description

Chrome: 58.0.3018.3
MR: 5817.227.0.0


See screenshots. On Mac and CrOS, there is a "Casting Video..." text. On Win, there is not.
 
mac_and_cros_remoting_icon.png
32.0 KB View Download
win_remoting_icon.PNG
21.1 KB View Download

Comment 1 by x...@chromium.org, Mar 3 2017

This doesn't repro on my windows workstation. I tried with same version of Chrome and MR as above.

Comment 2 by m...@chromium.org, Mar 3 2017

What versions of Windows does this repro or not repro on?

Comment 3 by x...@chromium.org, Mar 3 2017

It is Windows 7 on my workstation that doesn't repro this issue.

Comment 4 by x...@chromium.org, Mar 6 2017

Cc: m...@chromium.org
Owner: x...@chromium.org
Status: Started (was: Assigned)
Verified this repros on Windows 10 and Windows 8, but not Windows 7. Continue investigating.

Comment 5 by xjz@google.com, Mar 7 2017

Confirmed that the issue is caused by the failure calling SystemParametersInfoW()[1], which results a null typeface created by calling SkTypeface::MakeFromName()[2].

The issue doesn't repro with "--no-sandbox".

[1] https://cs.chromium.org/chromium/src/third_party/skia/src/ports/SkFontMgr_win_dw.cpp?rcl=488f0d674811ae038d34ece9d59c2824ebd4df8f&l=941

[2] https://cs.chromium.org/chromium/src/media/remoting/interstitial.cc?rcl=b761712bc735e72369c527fc4cd861b39e5c20cf&l=76

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://skia.googlesource.com/skia/+/c0128d4683d033f5250c35a22e1f857561b22a4b

commit c0128d4683d033f5250c35a22e1f857561b22a4b
Author: Ben Wagner <bungeman@google.com>
Date: Wed Mar 08 16:38:45 2017

DW last resort font default name not necessary.

Currently in SkFontMgr_DirectWrite when trying to find some last resort
font SystemParametersInfoW failing is considered a fatal error and so
onLegacyCreateTypeface will return nullptr. Instead, treat failure of
getDefaultFontFamily as ignorable and continue to the default default.

BUG= chromium:697672 

Change-Id: I1ea018627487fbd39b1d0eebad4c798346d09c94
Reviewed-on: https://skia-review.googlesource.com/9408
Reviewed-by: Ben Wagner <bungeman@google.com>
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>

[modify] https://crrev.com/c0128d4683d033f5250c35a22e1f857561b22a4b/src/ports/SkFontMgr_win_dw.cpp

Project Member

Comment 7 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bb4c517fc33e7d65dcaccea86bd9430cd6473ec4

commit bb4c517fc33e7d65dcaccea86bd9430cd6473ec4
Author: skia-deps-roller@chromium.org <skia-deps-roller@chromium.org>
Date: Wed Mar 08 20:42:10 2017

Roll src/third_party/skia/ b4dc549c9..598524df9 (18 commits)

https://skia.googlesource.com/skia.git/+log/b4dc549c9d12..598524df99bf

$ git log b4dc549c9..598524df9 --date=short --no-merges --format='%ad %ae %s'
2017-03-08 reed clip to elements directly, no need for replay indirection
2017-03-08 mtklein Revert "Turn on SkJumper all the time."
2017-03-08 brianosman Refactor GrColorSpaceXformHelper
2017-03-08 fmalita Fix SkJumper gcc warning
2017-03-08 robertphillips Add a unique ID to GrOpLists and return it from GrRenderTargetContext::addDrawOp
2017-03-08 bsalomon Make GPU dm sink correctly advertise whether it is multisampled
2017-03-08 msarett Add gm to compare premuls in dst vs. src color space
2017-03-08 reed special device for not drawing -- performs no clipping
2017-03-08 halcanary bin/fetch-clang-format
2017-03-08 halcanary formatting change: FuzzCanvas.cpp
2017-03-08 bungeman DW last resort font default name not necessary.
2017-03-06 halcanary SkPDF: Always get advances at unitsPerEm.
2017-03-06 liyuqian Remove SK_SUPPORT_LEGACY_AAA flag as chromium now turns it off
2017-03-08 bsalomon Use glXCreateContextAttribsARB in viewer to make it easier to attach RenderDoc.
2017-03-08 msarett Rename not-fBlendCorrectly to fNonLinearBlending
2017-03-08 msarett Optimize mipmap downsample_2_2 in sRGB mode
2017-03-07 mtklein Turn on SkJumper all the time.
2017-03-07 fmalita Refactor GrTextBlobCache

Created with:
  roll-dep src/third_party/skia
BUG= 697672 


Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, see:
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium#TOC-Failures-due-to-DEPS-rolls


CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
TBR=djsollen@chromium.org

Change-Id: Ie055e8ce4f3dffacb70d51cc8f47ee1af86a1ee8
Reviewed-on: https://chromium-review.googlesource.com/451679
Reviewed-by: Skia Deps Roller <skia-deps-roller@chromium.org>
Commit-Queue: Skia Deps Roller <skia-deps-roller@chromium.org>
Cr-Commit-Position: refs/heads/master@{#455534}
[modify] https://crrev.com/bb4c517fc33e7d65dcaccea86bd9430cd6473ec4/DEPS

Comment 8 by x...@chromium.org, Mar 8 2017

Cc: bunge...@chromium.org
Labels: -Pri-2 -M-57 M-58 Pri-1
bungeman@: With the above patch, now the text message is able to be displayed on Windows 8. But the ellipsis mark turns to a square on Windows 8.

The family name of the typeface returned by SkTypeface::MakeFromName("sans",...) is SimSun-Ext8.

Any ideas?
with-fix-remoting-message.png
24.1 KB View Download
Well, you're getting the default default font, which is just some font on your machine without any regard to anything. Are you requesting whatever the user set their default font to in settings? There is no magical font named 'sans'. How are you shaping this text? How are you doing font fallback? Where is the code doing the actual draw? If you're calling into Skia with anything other than kGlyphID_TextEncoding set on the paint / text blob you're probably holding it wrong.

Comment 10 by m...@chromium.org, Mar 8 2017

bungeman: I don't think this is an issue with how we're using the API (though, we'll check on that kGlyphID thing you mentioned). "sans" is something all platforms should provide a reasonably-close default sans-serif font for. In fact, on Win8/10, when running Chrome with --no-sandbox, "sans" does map to a perfectly good font. It also works on Win7 and all other platforms. This suggests the font-lookup mechanisms are broken because of the sandboxing: Perhaps the platform integration code is making calls into the OS library that fail while in a sandbox? How do we fix this? Why are Skia fonts broken, but not Webkit fonts? Do they use different integration w/ the OS?

FWIW, the Chrome Windows platform team pointed us as this: https://cs.chromium.org/chromium/src/content/child/dwrite_font_proxy/dwrite_font_proxy_win.h?rcl=8319b23bcb1895fea5a4a308837c100beb42f068&l=28 Unfortunately, the original author of that code has since left the Chromium project.

Comment 11 by m...@chromium.org, Mar 8 2017

Addendum to comment #10: To be clear, a reasonable font for "sans" is found and works *with* sandboxing turned on; on Win7, and all other platfroms.
Why do you think "sans" is the name of a valid font? This is a web browser-ism, the system doesn't have to have (and often doesn't have) any match for "sans". Even with FontConfig having a "sans" match is just a common convention. "sans" doesn't get you a sans-serif font or match anything on Windows 7, you were previously just getting whatever the default UI font as a fallback (because there was no "sans" font) which doesn't need to be sans-serif or even exist. When Blink needs a font for "sans" it uses the font set by the user in preferences. It will also do font fallback for any glyphs not in the current font.

Comment 13 by x...@chromium.org, Mar 10 2017

Cc: w...@chromium.org
Update: Tried using gfx::RenderText to render the message instead, but crashed on Windows 8.1 at this DCHECK: https://cs.chromium.org/chromium/src/base/win/win_util.cc?rcl=d8ee83729c29c767b7465dc23940b83a8a88fb89&l=300
Seems the same problem: sandbox blocks the access to SystemParametersInfo() on Windows 8.1.
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/df9b67eb9751650cebee13333ab125bb3f11df75

commit df9b67eb9751650cebee13333ab125bb3f11df75
Author: xjz <xjz@chromium.org>
Date: Thu Apr 13 23:28:25 2017

Media Remoting: Add interstitial elements to media element shadow dom.

This CL add the following elements to media element shadow dom when
media remoting starts.
1. Add a disable button to allow user stop remoting and fall back to
tab mirroring. This is to solve the accessiblity regression caused by
media remoting with lacking support for closed captions.
2. Add a background image, cast icon and a text message. These three
are currently rendered as a video frame in media renderer. Move them
to media element to keep consistent with the disabel button and solve
the font renderering issue on Windows 8/10.

The old interstitial (rendered as a video frame) will be removed in a
later CL.

BUG= 700572 ,  697672 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2767823002
Cr-Commit-Position: refs/heads/master@{#464604}

[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/content/app/strings/content_strings.grd
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/content/child/blink_platform_impl.cc
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/media/base/media_observer.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/css/CSSPrimitiveValueMappings.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/css/CSSValueKeywords.json5
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/css/mediaControls.css
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/dom/Node.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/BUILD.gn
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/HTMLMediaElement.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/HTMLVideoElement.cpp
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/HTMLVideoElement.h
[add] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.cpp
[add] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/shadow/MediaRemotingElements.h
[add] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/shadow/MediaRemotingInterstitial.cpp
[add] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/html/shadow/MediaRemotingInterstitial.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/layout/LayoutMedia.cpp
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/paint/MediaControlsPainter.cpp
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/paint/MediaControlsPainter.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/core/paint/ThemePainter.cpp
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/modules/media_controls/MediaControlsImpl.cpp
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/Source/platform/ThemeTypes.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/public/blink_image_resources.grd
[add] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/public/default_100_percent/blink/mediaremoting_cast.png
[add] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/public/default_200_percent/blink/mediaremoting_cast.png
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/public/platform/WebLocalizedString.h
[modify] https://crrev.com/df9b67eb9751650cebee13333ab125bb3f11df75/third_party/WebKit/public/platform/WebMediaPlayerClient.h

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2504c4da27a272a4906a7fd1bde697c8bdf7def1

commit 2504c4da27a272a4906a7fd1bde697c8bdf7def1
Author: xjz <xjz@chromium.org>
Date: Tue Apr 18 18:50:14 2017

Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG= 700572 ,  697672 

Review-Url: https://codereview.chromium.org/2801853002
Cr-Commit-Position: refs/heads/master@{#465303}

[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/chrome/app/generated_resources.grd
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/chrome/common/media/media_resource_provider.cc
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/base/localized_strings.h
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/base/media_observer.h
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/remoting/BUILD.gn
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/remoting/DEPS
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/remoting/courier_renderer.cc
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/remoting/courier_renderer.h
[delete] https://crrev.com/e3b1b256fd85fa0c846e49240e681d4f5cee91f7/media/remoting/interstitial.cc
[delete] https://crrev.com/e3b1b256fd85fa0c846e49240e681d4f5cee91f7/media/remoting/interstitial.h
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/remoting/renderer_controller.cc
[modify] https://crrev.com/2504c4da27a272a4906a7fd1bde697c8bdf7def1/media/remoting/renderer_controller.h

This should be resolved now, yes?
Project Member

Comment 17 by bugdroid1@chromium.org, Apr 20 2017

Labels: merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2828b8c2d6fd95fccb8145129deb99c33790ec80

commit 2828b8c2d6fd95fccb8145129deb99c33790ec80
Author: xjz <xjz@chromium.org>
Date: Thu Apr 20 20:01:17 2017

Merge M59: Media Remoting: Remove old interstitial implementation.

Remove old implementation that renders the interstitial on a video
frame. The new implemenation is committed:
https://codereview.chromium.org/2767823002/

BUG= 700572 ,  697672 
NOTRY=true
NOPRESUBMIT=true
TBR=miu@chromium.org,sandersd@chromium.org,jochen@chromium.org

Review-Url: https://codereview.chromium.org/2801853002
Cr-Commit-Position: refs/heads/master@{#465303}
(cherry picked from commit 2504c4da27a272a4906a7fd1bde697c8bdf7def1)

Review-Url: https://codereview.chromium.org/2829003004
Cr-Commit-Position: refs/branch-heads/3071@{#97}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/chrome/app/generated_resources.grd
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/chrome/common/media/media_resource_provider.cc
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/base/localized_strings.h
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/base/media_observer.h
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/blink/webmediaplayer_impl.h
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/remoting/BUILD.gn
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/remoting/DEPS
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/remoting/courier_renderer.cc
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/remoting/courier_renderer.h
[delete] https://crrev.com/cb1817af2f491075b8a594ea74255da02904ace9/media/remoting/interstitial.cc
[delete] https://crrev.com/cb1817af2f491075b8a594ea74255da02904ace9/media/remoting/interstitial.h
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/remoting/renderer_controller.cc
[modify] https://crrev.com/2828b8c2d6fd95fccb8145129deb99c33790ec80/media/remoting/renderer_controller.h

Comment 18 by x...@chromium.org, Apr 20 2017

Labels: -merge-merged-3071
Status: Fixed (was: Started)

Sign in to add a comment