Bug 105844 - [EFL][WK2] MiniBrowser could not be launched on specific machine
Summary: [EFL][WK2] MiniBrowser could not be launched on specific machine
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryuan Choi
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-28 19:20 PST by Kangil Han
Modified: 2012-12-30 17:48 PST (History)
7 users (show)

See Also:


Attachments
suggestion (2.57 KB, patch)
2012-12-28 20:26 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kangil Han 2012-12-28 19:20:52 PST
Launching MibiBrowser doesn't work on my machine because executable path of WebProcess is NULL in child process.
Comment 1 Ryuan Choi 2012-12-28 20:26:14 PST
Created attachment 180910 [details]
suggestion
Comment 2 Kangil Han 2012-12-28 20:55:31 PST
FOA, MiniBrowser works with your suggestion, thanks. :)
Would you please mention in ChangeLog why we should use CString instead of const char* in detail?
Please name an attachment as a patch as well. :)
Comment 3 Gyuyoung Kim 2012-12-28 21:20:02 PST
Comment on attachment 180910 [details]
suggestion

As Kangil said, it would be good if you say why we need to use CString instead of const char*. However, I think we have to use CString inside WebKit.
Comment 4 WebKit Review Bot 2012-12-28 21:31:51 PST
Comment on attachment 180910 [details]
suggestion

Clearing flags on attachment: 180910

Committed r138557: <http://trac.webkit.org/changeset/138557>
Comment 5 WebKit Review Bot 2012-12-28 21:31:55 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 Thiago Marcos P. Santos 2012-12-30 04:46:26 PST
Comment on attachment 180910 [details]
suggestion

View in context: https://bugs.webkit.org/attachment.cgi?id=180910&action=review

> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:48
> -        executablePath = executablePathOfWebProcess().utf8().data();
> +        executablePath = executablePathOfWebProcess().utf8();

Now I see the issue here. A better fix would be make executablePathOfWebProcess() return a const reference instead of a copy, since it is getting static data.
Comment 7 Chris Dumez 2012-12-30 04:49:30 PST
Comment on attachment 180910 [details]
suggestion

View in context: https://bugs.webkit.org/attachment.cgi?id=180910&action=review

>> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:48
>> +        executablePath = executablePathOfWebProcess().utf8();
> 
> Now I see the issue here. A better fix would be make executablePathOfWebProcess() return a const reference instead of a copy, since it is getting static data.

Thiago, the issue is that String::utf8() returns a new temporary CString object and CString::data() returns a pointer to the internal CString data representation. Since the CString is temporary, the pointer returned by data() becomes invalid on next line.
Comment 8 Thiago Marcos P. Santos 2012-12-30 05:15:46 PST
Comment on attachment 180910 [details]
suggestion

View in context: https://bugs.webkit.org/attachment.cgi?id=180910&action=review

>>> Source/WebKit2/UIProcess/Launcher/efl/ProcessLauncherEfl.cpp:48
>>> +        executablePath = executablePathOfWebProcess().utf8();
>> 
>> Now I see the issue here. A better fix would be make executablePathOfWebProcess() return a const reference instead of a copy, since it is getting static data.
> 
> Thiago, the issue is that String::utf8() returns a new temporary CString object and CString::data() returns a pointer to the internal CString data representation. Since the CString is temporary, the pointer returned by data() becomes invalid on next line.

Now I see, this patch is fine. Sorry for introducing this bug and thanks guys for fixing.
Comment 9 Kangil Han 2012-12-30 17:48:12 PST
(In reply to comment #7)
> Thiago, the issue is that String::utf8() returns a new temporary CString object and CString::data() returns a pointer to the internal CString data representation. Since the CString is temporary, the pointer returned by data() becomes invalid on next line.

Yes, right! thanks for explanation Chris! :)

(In reply to comment #8)
> Now I see, this patch is fine. Sorry for introducing this bug and thanks guys for fixing.

No problem! :)