Bug 37277

Summary: Move the IDB::open ExceptionCode parameter to be last
Product: WebKit Reporter: Jeremy Orlow <jorlow>
Component: New BugsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, darin, fishd, japhet
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch none

Description Jeremy Orlow 2010-04-08 08:45:29 PDT
Move the IDB::open ExceptionCode paramter to be last
Comment 1 Jeremy Orlow 2010-04-08 08:50:09 PDT
Created attachment 52873 [details]
Patch
Comment 2 Darin Adler 2010-04-09 09:27:41 PDT
Normally we use the typedef ExceptionCode so we don't have to name exception code arguments. Please do that whenever practical.
Comment 3 Jeremy Orlow 2010-04-09 09:30:52 PDT
(In reply to comment #2)
> Normally we use the typedef ExceptionCode so we don't have to name exception
> code arguments. Please do that whenever practical.

All the uses of int are in the Chromium WebKit API.  I suppose maybe we should add a WebExceptionCode class there, but I also don't see a huge advantage to doing so.

+ fishd in case we think it's a good idea to do so.
Comment 4 Darin Adler 2010-04-09 09:39:35 PDT
(In reply to comment #3)
> All the uses of int are in the Chromium WebKit API.  I suppose maybe we should
> add a WebExceptionCode class there, but I also don't see a huge advantage to
> doing so.

I'm surprised that ExceptionCode is part of the Chromium WebKit API. It's an implementation detail we've often changed around in the past and not something particularly stable, so not great for API. But anyway, not on point for this patch.
Comment 5 Jeremy Orlow 2010-04-09 09:43:35 PDT
(In reply to comment #4)
> (In reply to comment #3)
> > All the uses of int are in the Chromium WebKit API.  I suppose maybe we should
> > add a WebExceptionCode class there, but I also don't see a huge advantage to
> > doing so.
> 
> I'm surprised that ExceptionCode is part of the Chromium WebKit API. It's an
> implementation detail we've often changed around in the past and not something
> particularly stable, so not great for API. But anyway, not on point for this
> patch.

Did you mean you're surprised it's not?

Anyway, I guess you're right that it is an implementation detail that should probably be abstracted away by an API.  I'll look into changing this, unless you have an objection, fishd.
Comment 6 Darin Adler 2010-04-09 09:45:14 PDT
(In reply to comment #5)
> > I'm surprised that ExceptionCode is part of the Chromium WebKit API. It's an
> > implementation detail we've often changed around in the past and not something
> > particularly stable, so not great for API. But anyway, not on point for this
> > patch.
> 
> Did you mean you're surprised it's not?

I am surprised that WebCore's internal representation of DOM exceptions is a part of the API. Not the ExceptionCode typedef, which, as you say, is not.
Comment 7 WebKit Commit Bot 2010-04-09 12:50:47 PDT
Comment on attachment 52873 [details]
Patch

Clearing flags on attachment: 52873

Committed r57350: <http://trac.webkit.org/changeset/57350>
Comment 8 WebKit Commit Bot 2010-04-09 12:50:53 PDT
All reviewed patches have been landed.  Closing bug.