Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(844)

Issue 551873004: Revert of Add FontFaceDescriptors IDL dictionary (Closed)

Created:
6 years, 3 months ago by keishi
Modified:
6 years, 3 months ago
CC:
blink-reviews, arv+blink, blink-reviews-css, ed+blinkwatch_opera.com, abarth-chromium, dglazkov+blink, blink-reviews-bindings_chromium.org, Inactive, darktears, apavlov+blink_chromium.org, rwlbuis, rune+blink
Base URL:
https://chromium.googlesource.com/chromium/blink.git@idl_gyp
Project:
blink
Visibility:
Public.

Description

Revert of Add FontFaceDescriptors IDL dictionary (patchset #4 id:60001 of https://codereview.chromium.org/481453003/) Reason for revert: Broke build http://build.chromium.org/p/chromium.webkit/builders/GPU%20Mac%20Builder/builds/19956 Original issue's description: > Add FontFaceDescriptors IDL dictionary > > To follow the spec[1]. This is the first place to use IDL dictionary > execept for testing, so this CL also includes build related changes. > Existing tests(e.g. fast/css/fontface-constructor-error.html) should > cover this change. > > [1] http://dev.w3.org/csswg/css-font-loading/#fontface-interface > > BUG=403150 > > Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181610 TBR=haraken@chromium.org,ksakamoto@chromium.org,bashi@chromium.org NOTREECHECKS=true NOTRY=true BUG=403150 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181612

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -67 lines) Patch
M Source/bindings/core/idl.gni View 2 chunks +2 lines, -6 lines 0 comments Download
M Source/bindings/core/idl.gypi View 3 chunks +4 lines, -8 lines 0 comments Download
M Source/bindings/core/v8/BUILD.gn View 2 chunks +7 lines, -4 lines 0 comments Download
M Source/bindings/core/v8/generated.gyp View 2 chunks +4 lines, -2 lines 0 comments Download
M Source/core/BUILD.gn View 1 chunk +0 lines, -2 lines 0 comments Download
M Source/core/core.gni View 2 chunks +0 lines, -4 lines 0 comments Download
M Source/core/core.gyp View 1 chunk +2 lines, -1 line 0 comments Download
M Source/core/core.gypi View 1 chunk +12 lines, -7 lines 0 comments Download
M Source/core/css/FontFace.h View 3 chunks +4 lines, -5 lines 0 comments Download
M Source/core/css/FontFace.cpp View 5 chunks +18 lines, -11 lines 0 comments Download
M Source/core/css/FontFace.idl View 1 chunk +3 lines, -3 lines 0 comments Download
D Source/core/css/FontFaceDescriptors.idl View 1 chunk +0 lines, -14 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
keishi
Created Revert of Add FontFaceDescriptors IDL dictionary
6 years, 3 months ago (2014-09-09 06:40:41 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/keishi@chromium.org/551873004/1
6 years, 3 months ago (2014-09-09 06:41:24 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as 181612
6 years, 3 months ago (2014-09-09 06:41:51 UTC) #3
haraken
LGTM
6 years, 3 months ago (2014-09-09 06:45:04 UTC) #4
bashi
On 2014/09/09 06:45:04, haraken wrote: > LGTM Sorry.. Thank you for reverting.
6 years, 3 months ago (2014-09-09 07:16:50 UTC) #5
bashi
On 2014/09/09 07:16:50, bashi1 wrote: > On 2014/09/09 06:45:04, haraken wrote: > > LGTM > ...
6 years, 3 months ago (2014-09-09 08:10:18 UTC) #6
keishi
On 2014/09/09 08:10:18, bashi1 wrote: > On 2014/09/09 07:16:50, bashi1 wrote: > > On 2014/09/09 ...
6 years, 3 months ago (2014-09-09 08:33:43 UTC) #7
bashi
6 years, 3 months ago (2014-09-09 08:44:00 UTC) #8
Message was sent while issue was closed.
On 2014/09/09 08:33:43, keishi wrote:
> On 2014/09/09 08:10:18, bashi1 wrote:
> > On 2014/09/09 07:16:50, bashi1 wrote:
> > > On 2014/09/09 06:45:04, haraken wrote:
> > > > LGTM
> > > 
> > > Sorry.. Thank you for reverting.
> > 
> > Umm. I can't reproduce the link error. I ran a tryjob on mac_gpu in
> > https://codereview.chromium.org/556543003
> >  and the job successfully finished.
> > 
> > The original error message was:
> > Undefined symbols for architecture i386:
> >   "blink::FontFace::create(blink::ExecutionContext*, WTF::AtomicString
const&,
> > WTF::String const&, blink::FontFaceDescriptors const*)", ref\
> > erenced from:
> >      
> > blink::FontFaceV8Internal::constructor1(v8::FunctionCallbackInfo<v8::Value>
> > const&) in libwebcore_generated.a(webcore_generated.V8Gen\
> > eratedCoreBindings05.o)
> > 
> > ...and the CL contains the function in FontFace.cpp. I looked over the log
> >
>
(http://build.chromium.org/p/chromium.webkit/builders/GPU%20Mac%20Builder/buil...),
> > but I couldn't find FontFace.cpp got recompiled. Maybe something went wrong
on
> > the bot?
> 
> I'll keep an eye on it so you can reland if you want to.

Thank you! I've prepared the re-land CL.
https://codereview.chromium.org/556543003/

Powered by Google App Engine
This is Rietveld 408576698