implement Serialize/Deserialize, so we can serialize pictures

implement Serialize/Deserialize, so we can serialize pictures
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=150794

[reed1, 2012-08-02 18:19:57]

Can you install this locally, to see if it fixes anything?

[reed1, 2012-08-02 18:21:54]

the acid test being:
- record on linux, playback on win/mac

[alokp, 2012-08-02 18:26:05]

Daniel: Could you please try this patch and confirm if it fixes the crash in
SkFontHost::serialize()?

[reed1, 2012-08-02 19:02:34]

sorry, first patch was bogus. Try this version.

[dmurph, 2012-08-02 20:21:28]

On 2012/08/02 19:02:34, reed1 wrote:
> sorry, first patch was bogus. Try this version.

Hm, that crashes the renderer without a stacktrace or anything.  Weird.

[reed1, 2012-08-02 21:19:29]

I also crash in release, but in debug I see this


[006:043] Error(scriptinterface.cc:497): Can't send message - onmessage callback
not set
[12592:12592:693251980435:ERROR:renderer_main.cc(203)] Running without renderer
sandbox
[12578:12578:693302804088:FATAL:file_path.cc(454)] Check failed:
!IsPathAbsolute(component). 
Backtrace:
	base::debug::StackTrace::StackTrace() [0x7fe5a767884a]
	logging::LogMessage::~LogMessage() [0x7fe5a76a0917]
	FilePath::Append() [0x7fe5a7683754]
	FilePath::AppendASCII() [0x7fe5a76839c5]
	content::GpuBenchmarkingWrapper::PrintToSkPicture() [0x7fe5a7f11d10]
	v8::internal::HandleApiCallHelper<>() [0x7fe5a84ca8d8]
	v8::internal::Builtin_Impl_HandleApiCall() [0x7fe5a84c56f1]
	v8::internal::Builtin_HandleApiCall() [0x7fe5a84c56c2]
	0x24dee780618e

Here is what I entered int eh JS console:
> chrome.gpuBenchmarking.printToSkPicture("/usr/local/google/picts/")

... but I got the file!

[chudy, 2012-08-02 21:22:24]

That's the same stack trace I'm getting but no file on my end appears
in Linux debug

Sent from my iPhone

On Aug 2, 2012, at 5:19 PM, "reed@google.com" <reed@google.com> wrote:

>
> I also crash in release, but in debug I see this
>
>
> [006:043] Error(scriptinterface.cc:497): Can't send message - onmessage
callback
> not set
> [12592:12592:693251980435:ERROR:renderer_main.cc(203)] Running without
renderer
> sandbox
> [12578:12578:693302804088:FATAL:file_path.cc(454)] Check failed:
> !IsPathAbsolute(component).
> Backtrace:
>    base::debug::StackTrace::StackTrace() [0x7fe5a767884a]
>    logging::LogMessage::~LogMessage() [0x7fe5a76a0917]
>    FilePath::Append() [0x7fe5a7683754]
>    FilePath::AppendASCII() [0x7fe5a76839c5]
>    content::GpuBenchmarkingWrapper::PrintToSkPicture() [0x7fe5a7f11d10]
>    v8::internal::HandleApiCallHelper<>() [0x7fe5a84ca8d8]
>    v8::internal::Builtin_Impl_HandleApiCall() [0x7fe5a84c56f1]
>    v8::internal::Builtin_HandleApiCall() [0x7fe5a84c56c2]
>    0x24dee780618e
>
> Here is what I entered int eh JS console:
>> chrome.gpuBenchmarking.printToSkPicture("/usr/local/google/picts/")
>
> ... but I got the file!
>
> https://chromiumcodereview.appspot.com/10837077/

[alokp, 2012-08-02 21:24:29]

I will look into this when I get back to my desk tomorrow.

[reed1, 2012-08-08 17:26:15]

Alok, have you already incorporated this or a similar change into your
patch(es)?

[alokp, 2012-08-08 17:33:57]

On 2012/08/08 17:26:15, reed1 wrote:
> Alok, have you already incorporated this or a similar change into your
> patch(es)?

I have not tried or incorporated this patch yet.

I was looking at other crashes on linux:
- I could not repro FATAL:file_path.cc(454)] Check failed:
!IsPathAbsolute(component).
- http://codereview.chromium.org/10823129 should fix another one.

Is this ready to be checked in?

[reed1, 2012-08-08 17:38:35]

I think so (and am issuing a fresh set of trybots). I was hoping to have a lgtm
from someone else who had tried it out (in conjunction with your patch/tool).

[alokp, 2012-08-08 21:11:34]

lgtm with minor style nits.

https://chromiumcodereview.appspot.com/10837077/diff/7005/skia/ext/SkFontHost...
File skia/ext/SkFontHost_fontconfig.cpp (right):

https://chromiumcodereview.appspot.com/10837077/diff/7005/skia/ext/SkFontHost...
skia/ext/SkFontHost_fontconfig.cpp:34: #include
"../../src/ports/SkFontDescriptor.h"
Is this allowed by style guide?
you should replace this with third_party/skia/src/ports/SkFontDescriptor.h

https://chromiumcodereview.appspot.com/10837077/diff/7005/skia/ext/SkFontHost...
skia/ext/SkFontHost_fontconfig.cpp:215: // webkit handles fallbacks, we don't
unnecessary change.

[reed1, 2012-08-08 21:32:24]

address nits