Bug 37277 - Move the IDB::open ExceptionCode parameter to be last
Summary: Move the IDB::open ExceptionCode parameter to be last
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-08 08:45 PDT by Jeremy Orlow
Modified: 2010-04-09 12:50 PDT (History)
4 users (show)

See Also:


Attachments
Patch (10.63 KB, patch)
2010-04-08 08:50 PDT, Jeremy Orlow
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.