Bug 34705

Summary:

[BREWMP] Add a function to create a BREW instance without local variable declarations.

Product:

WebKit

Reporter:

Kwang Yul Seo <skyul>

Component:

Web Template Framework

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, beergun, commit-queue, darin, eric

Priority:

Version:

528+ (Nightly build)

Hardware:

Other

OS:

Other

Bug Depends on:

Bug Blocks:

33564

Attachments:

Description	Flags
Patch	none
Patch	eric: review-
Patch	none

Kwang Yul Seo

Reported 2010-02-08 08:02:50 PST

It is too verbose to create a BREW instance. For example, to create a IFileMgr instance, it takes three lines: IFileMgr* fileMgr; IShell* shell = reinterpret_cast<AEEApplet*>(GETAPPINSTANCE())->m_pIShell; ISHELL_CreateInstance(shell, AEECLSID_FILEMGR, reinterpret_cast<void**>(&fileMgr)); This pattern is repeated all over the code, so we need a shortcut here. Add a template function to create a BREW instance in one line. IFileMgr* fileMgr = createInstanceBrew<IFileMgr>(AEECLSID_FILEMGR); This can be nicely combined with OwnPtrBrew. OwnPtrBrew<IFileMgr> fileMgr(createInstanceBrew<IFileMgr>(AEECLSID_FILEMGR));

Attachments
Patch (3.05 KB, patch) 2010-02-08 08:08 PST, Kwang Yul Seo	no flags	Details Formatted Diff Diff
Patch (3.03 KB, patch) 2010-02-08 09:34 PST, Kwang Yul Seo	eric: review-	Details Formatted Diff Diff
Patch (3.03 KB, patch) 2010-03-07 01:34 PST, Kwang Yul Seo	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Kwang Yul Seo

Comment 1 2010-02-08 08:08:31 PST

Created attachment 48333 [details] Patch ShellUtilBrew.h is currently under wtf/brew, but I am not sure if this is the right place to put platform-specific utility functions.

Darin Adler

Comment 2 2010-02-08 09:13:06 PST

(In reply to comment #1) > ShellUtilBrew.h is currently under wtf/brew, but I am not sure if this is the > right place to put platform-specific utility functions. It’s the right directory for low-level functions of this type. But “utility” is not a word we ever use, and we certainly frown on “Util” in a file name.

Kwang Yul Seo

Comment 3 2010-02-08 09:34:35 PST

Created attachment 48338 [details] Patch Remove Util from the filename and prefix the function with shell createInstanceBrew -> shellCreateInstanceBrew

Eric Seidel (no email)

Comment 4 2010-02-08 19:20:10 PST

Should this return a PassOwnPtrBrew?

Kwang Yul Seo

Comment 5 2010-02-08 19:46:50 PST

(In reply to comment #4) > Should this return a PassOwnPtrBrew? These is no PassOwnPtrBrew class. I just wanted to simplify the creation of a BREW instance. As OwnPtrBrew takes a raw pointer in the constructor, we can use an idiom like the following: OwnPtrBrew<IFileMgr> fileMgr(createInstanceBrew<IFileMgr>(AEECLSID_FILEMGR)); Do you suggest that this should return a PassOwnPtrBrew?

Adam Barth

Comment 6 2010-02-09 12:40:40 PST

> Do you suggest that this should return a PassOwnPtrBrew? In general, createMumble methods return a PassFooBar object to ensure we don't leak the allocated memory.

Eric Seidel (no email)

Comment 7 2010-02-17 15:54:47 PST

Comment on attachment 48338 [details] Patch Why would this be part of WTF? This seems like port-specific goop. We don't need to have PassOwnPtrBrew to approve this, but eventually it would be nice to have such a class to make leaking memory returned from this function impossible.

Kwang Yul Seo

Comment 8 2010-03-07 01:34:28 PST

Created attachment 50169 [details] Patch Make the function return PassOwnPtr. This prevents memory leaking.

Eric Seidel (no email)

Comment 9 2010-03-15 16:21:30 PDT

Comment on attachment 50169 [details] Patch LGTM.

WebKit Commit Bot

Comment 10 2010-03-15 21:07:06 PDT

Comment on attachment 50169 [details] Patch Clearing flags on attachment: 50169 Committed r56031: <http://trac.webkit.org/changeset/56031>

WebKit Commit Bot

Comment 11 2010-03-15 21:07:13 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.