Bug 69450

Summary: [EFL] DRT: Create icon database path in LayoutTestController::setIconDatabaseEnabled.
Product: WebKit Reporter: Raphael Kubo da Costa (:rakuco) <rakuco>
Component: WebKit EFLAssignee: Raphael Kubo da Costa (:rakuco) <rakuco>
Status: RESOLVED FIXED    
Severity: Normal CC: gyuyoung.kim, leandro, lucas.de.marchi, rniwa, tonikitoo, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Follow rniwa's suggestions
none
Er, no need to ask for review again. List rniwa as the reviewer
none
Follow rniwa's new suggestion none

Raphael Kubo da Costa (:rakuco)
Reported 2011-10-05 11:59:37 PDT
[EFL] DRT: Create icon database path in LayoutTestController::setIconDatabaseEnabled.
Attachments
Patch (2.35 KB, patch)
2011-10-05 12:00 PDT, Raphael Kubo da Costa (:rakuco)
no flags
Follow rniwa's suggestions (2.31 KB, patch)
2011-10-06 11:44 PDT, Raphael Kubo da Costa (:rakuco)
no flags
Er, no need to ask for review again. List rniwa as the reviewer (2.31 KB, patch)
2011-10-06 11:52 PDT, Raphael Kubo da Costa (:rakuco)
no flags
Follow rniwa's new suggestion (2.22 KB, patch)
2011-10-06 11:54 PDT, Raphael Kubo da Costa (:rakuco)
no flags
Raphael Kubo da Costa (:rakuco)
Comment 1 2011-10-05 12:00:49 PDT
Ryosuke Niwa
Comment 2 2011-10-06 11:37:51 PDT
Comment on attachment 109833 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109833&action=review It'll be nice if the logic to obtain the temp dir. was extracted as a function. > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:403 > + char* tempDir = getenv("TMPDIR"); You should use const char* instead. > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:409 > + else { > + tempDir = getenv("TEMP"); > + if (tempDir) You should define the variable inside if as in: else if (tempDir = getenv("TEMP"))
Raphael Kubo da Costa (:rakuco)
Comment 3 2011-10-06 11:44:46 PDT
Created attachment 109986 [details] Follow rniwa's suggestions
Ryosuke Niwa
Comment 4 2011-10-06 11:47:17 PDT
Comment on attachment 109986 [details] Follow rniwa's suggestions View in context: https://bugs.webkit.org/attachment.cgi?id=109986&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:408 > + else { > + if (tempDir = getenv("TEMP")) Please use else if instead of nesting like this.
Raphael Kubo da Costa (:rakuco)
Comment 5 2011-10-06 11:52:02 PDT
Created attachment 109988 [details] Er, no need to ask for review again. List rniwa as the reviewer
Ryosuke Niwa
Comment 6 2011-10-06 11:53:05 PDT
Comment on attachment 109988 [details] Er, no need to ask for review again. List rniwa as the reviewer View in context: https://bugs.webkit.org/attachment.cgi?id=109988&action=review > Tools/DumpRenderTree/efl/LayoutTestControllerEfl.cpp:408 > + else { > + if (tempDir = getenv("TEMP")) Please fix this before landing it.
Raphael Kubo da Costa (:rakuco)
Comment 7 2011-10-06 11:54:57 PDT
Created attachment 109990 [details] Follow rniwa's new suggestion
WebKit Review Bot
Comment 8 2011-10-06 12:27:58 PDT
Comment on attachment 109990 [details] Follow rniwa's new suggestion Clearing flags on attachment: 109990 Committed r96848: <http://trac.webkit.org/changeset/96848>
WebKit Review Bot
Comment 9 2011-10-06 12:28:03 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.