add font cache size to about:support

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: jtd, Assigned: jtd)

Tracking

Trunk
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 1 obsolete attachment)

2.44 KB, patch
bas
: review+
Joe Drew (not getting mail)
: review+
Joe Drew (not getting mail)
: approval2.0+
Details | Diff | Splinter Review
866 bytes, patch
Joe Drew (not getting mail)
: review+
Joe Drew (not getting mail)
: approval2.0+
Details | Diff | Splinter Review
775 bytes, patch
jfkthame
: review+
Joe Drew (not getting mail)
: approval2.0+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
Created attachment 507795 [details] [diff] [review]
patch, add font cache size to about:support

Along with the version of DirectWrite, the size of the font cache on a machine is an important cause of bug 602792, so it would be very useful to have this info when diagnosing startup problems on a given users machine.

This patch just adds it to the DirectWrite version field.
Attachment #507795 - Flags: review?(bas.schouten)
Comment on attachment 507795 [details] [diff] [review]
patch, add font cache size to about:support

Looks fine to me. But it involves strings in some way, asking joe if he agrees.
Attachment #507795 - Flags: review?(joe)
Attachment #507795 - Flags: review?(bas.schouten)
Attachment #507795 - Flags: review+
Comment on attachment 507795 [details] [diff] [review]
patch, add font cache size to about:support

this doesn't require any strings, so it's fine

I'm not overjoyed about any hardcoded, untranslateable strings in our source code; I'd rather we just appended the number itself. But I don't feel strongly about that.
Attachment #507795 - Flags: review?(joe) → review+
(Assignee)

Comment 3

7 years ago
Comment on attachment 507795 [details] [diff] [review]
patch, add font cache size to about:support

Simple, low-impact change needed for debugging user problems with cold startup.

Right, hard-coded strings aren't so hot but it's a simple way to get debug info needed into the page.  My guess is that MS ships a fix for this problem so that we can simply omit this info in Firefox 4.next.
Attachment #507795 - Flags: approval2.0?
Attachment #507795 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 4

7 years ago
Landed on trunk
http://hg.mozilla.org/mozilla-central/rev/c4bbf11d4c54
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 5

7 years ago
This check-in apparently broke my local build on Windows XP, that I'm doing
using one of the supported configurations. Backing the commit out locally
resolved the problem. I built with a clean objdir. Here is the information
I gathered, let me know if you need any other piece of information.


gfxWindowsPlatform.obj : error LNK2019: unresolved external symbol __imp__PathAppendW@8 referenced in function "public: static void __cdecl gfxWindowsPlatform::GetFontCacheSize(class nsAString_internal &)" (?GetFontCacheSize@gfxWindowsPlatform@@SAXAAVnsAString_internal@@@Z)

gfxWindowsPlatform.obj : error LNK2019: unresolved external symbol __imp__SHGetFolderPathW@20 referenced in function "public: static void __cdecl gfxWindowsPlatform::GetFontCacheSize(class nsAString_internal &)" (?GetFontCacheSize@gfxWindowsPlatform@@SAXAAVnsAString_internal@@@Z)


Build environment:

Windows XP
Visual C++ 2008 Express
Windows 7 SDK
Windows Server 2003 R2 Platform SDK
MozillaBuild 1.5


.mozconfig options:

--enable-debug
--disable-optimize
--disable-static
--disable-libxul
--disable-ipc
--enable-chrome-format=jar
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Paolo, please file a new bug about the libxul build being broken.
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED

Updated

7 years ago
Blocks: 630328
No longer blocks: 630328
Depends on: 630328

Comment 7

7 years ago
Comment on attachment 507795 [details] [diff] [review]
patch, add font cache size to about:support

>+    file = FindFirstFileW(path, &findFileData);
You leak this file search handle because you forgot to FindClose it.

Updated

7 years ago
Depends on: 630844
(Assignee)

Comment 8

7 years ago
Created attachment 509639 [details] [diff] [review]
follow-up patch, fix leak and compiler warnings
Attachment #509639 - Flags: review?(joe)
Attachment #509639 - Flags: approval2.0?
Comment on attachment 509639 [details] [diff] [review]
follow-up patch, fix leak and compiler warnings

Can you use sizeof(size) instead of the literal 256 there?
Attachment #509639 - Flags: review?(joe)
Attachment #509639 - Flags: review+
Attachment #509639 - Flags: approval2.0?
Attachment #509639 - Flags: approval2.0+
(Assignee)

Comment 10

7 years ago
Landed follow-up patch on trunk
http://hg.mozilla.org/mozilla-central/rev/26d3a5196a5e
(Assignee)

Comment 11

7 years ago
Created attachment 512104 [details] [diff] [review]
patch, fix screwup

Joe Drew asked that I use sizeof() instead of an arbitrary constant but, argh, it turns out swprintf_s actually takes a *count* value rather than a size in number of bytes, so sizeof(size) isn't correct.
Attachment #512104 - Flags: review?(bas.schouten)
Attachment #512104 - Flags: review?(bas.schouten) → review+
(Assignee)

Updated

7 years ago
Attachment #512104 - Flags: approval2.0?
Comment on attachment 512104 [details] [diff] [review]
patch, fix screwup

(excuse the drive-by comment....)

Isn't this what NS_ARRAY_LENGTH(size) is for?
(Assignee)

Comment 13

7 years ago
Created attachment 512107 [details] [diff] [review]
patch, fix screwup v2
Attachment #512104 - Attachment is obsolete: true
Attachment #512107 - Flags: review?(jfkthame)
Attachment #512107 - Flags: approval2.0?
Attachment #512104 - Flags: approval2.0?
Comment on attachment 512107 [details] [diff] [review]
patch, fix screwup v2

Looks good to me.
Attachment #512107 - Flags: review?(jfkthame) → review+
Comment on attachment 512107 [details] [diff] [review]
patch, fix screwup v2

ffffuuuuu review fail
Attachment #512107 - Flags: approval2.0? → approval2.0+
(Assignee)

Comment 16

7 years ago
Landed fix screwup patch
https://bugzilla.mozilla.org/show_bug.cgi?id=626611

Comment 17

7 years ago
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b12pre) Gecko/20110214 Firefox/4.0b12pre ID:20110214030347
My about:support say "font cache n/a", is this correct behavior?

Graphics
  Adapter Description: ATI Radeon HD 4300/4500 Series
  Vendor ID: 1002
  Device ID: 954f
  Adapter RAM: 512
  Adapter Drivers: aticfx64 aticfx64 aticfx32 aticfx32 atiumd64 atidxx64 atiumdag atidxx32 atiumdva atiumd6a atitmm64
  Driver Version: 8.812.0.0
  Driver Date: 1-4-2011
  Direct2D Enabled: true
  DirectWrite Enabled: true (6.1.7600.20830, font cache n/a)
  WebGL Renderer: Google Inc. -- ANGLE -- OpenGL ES 2.0 (ANGLE 0.0.0.541)
  GPU Accelerated Windows: 1/1 Direct3D 10
(Assignee)

Comment 18

7 years ago
(In reply to comment #17)

> My about:support say "font cache n/a", is this correct behavior?

The "n/a" just signifies that there's no font cache file on your machine that matches:

  <windows>\ServiceProfiles\LocalService\AppData\Local\FontCache-*-*.dat

Are there any large files in that directory?

As noted in the description, having information about the size of the font cache is useful for debugging startup-related problems.  It's not required for any program functionality.

Comment 19

7 years ago
Inadittion to the comment #17.
I confirmed my Windows 7 has  C:\Windows\ServiceProfiles\LocalService\AppData\Local\FontCache-*.dat file.

ドライブ C のボリューム ラベルがありません。
 ボリューム シリアル番号は 3C63-6B61 です

 C:\Windows\ServiceProfiles\LocalService\AppData\Local のディレクトリ

2011/02/15  05:55    <DIR>          .
2011/02/15  05:55    <DIR>          ..
2011/02/05  02:24         1,116,232 FontCache-S-1-5-21-4009116227-806356473-3186492530-1000-12288.dat
2010/11/30  10:00         6,066,892 FontCache-S-1-5-21-4009116227-806356473-3186492530-1000-4096.dat
2011/02/15  13:51        23,438,364 FontCache-S-1-5-21-4009116227-806356473-3186492530-1000-8192.dat
2011/02/15  05:54        37,273,080 FontCache-S-1-5-21-4009116227-806356473-3186492530-500-12288.dat
2011/02/15  13:51           558,296 FontCache-System.dat
2011/02/15  05:54         2,956,784 FontCache3.0.0.0.dat
2009/07/14  13:45    <DIR>          Microsoft
2010/09/03  01:18    <DIR>          PnrpSqm
2011/02/10  01:52    <DIR>          Temp
               6 個のファイル          71,409,648 バイト
               5 個のディレクトリ  113,492,922,368 バイトの空き領域
(Assignee)

Comment 20

7 years ago
(In reply to comment #19)
> 2011/02/05  02:24         1,116,232
> FontCache-S-1-5-21-4009116227-806356473-3186492530-1000-12288.dat
> 2010/11/30  10:00         6,066,892
> FontCache-S-1-5-21-4009116227-806356473-3186492530-1000-4096.dat
> 2011/02/15  13:51        23,438,364
> FontCache-S-1-5-21-4009116227-806356473-3186492530-1000-8192.dat
> 2011/02/15  05:54        37,273,080
> FontCache-S-1-5-21-4009116227-806356473-3186492530-500-12288.dat

Interesting, the code should pick up the size of the last one in that list but let me verify that.

Comment 21

7 years ago
(In reply to comment #20)

> Interesting, the code should pick up the size of the last one in that list but
> let me verify that.

It seems that the user except the administrator cannot get file size because access to the directory is limited.

(Comment #19 shows dir-outputs in "a command prompt as Administrator")
(Assignee)

Comment 22

7 years ago
(In reply to comment #21)
> (In reply to comment #20)
>
> It seems that the user except the administrator cannot get file size because
> access to the directory is limited.
> 
> (Comment #19 shows dir-outputs in "a command prompt as Administrator")

Ah, that makes complete sense.

Updated

7 years ago
Blocks: 634286
You need to log in before you can comment on or make changes to this bug.