Bug 105844 – [EFL][WK2] MiniBrowser could not be launched on specific machine

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Kangil Han]

Launching MibiBrowser doesn't work on my machine because executable path of WebProcess is NULL in child process.

[Ryuan Choi]

Created attachment 180910 [details]
suggestion

[Kangil Han]

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. :)

[Gyuyoung Kim]

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.

[WebKit Review Bot]

Comment on attachment 180910 [details]
suggestion

Clearing flags on attachment: 180910

Committed r138557: <http://trac.webkit.org/changeset/138557>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Thiago Marcos P. Santos]

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.

[Chris Dumez]

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.

[Thiago Marcos P. Santos]

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.

[Kangil Han]

(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! :)