NEW75608
ResourceHandle::d should be renamed ResourceHandle::m_internal 
https://bugs.webkit.org/show_bug.cgi?id=75608
Summary ResourceHandle::d should be renamed ResourceHandle::m_internal
Hajime Morrita
Reported 2012-01-05 00:54:56 PST
"d" is such a horrible name....
Attachments
Patch (85.95 KB, patch)
2012-01-05 01:19 PST, Hajime Morrita 		no flags
DetailsFormatted DiffDiff
Patch (86.29 KB, patch)
2012-01-05 02:47 PST, Hajime Morrita 		gyuyoung.kim: commit-queue-
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1 2012-01-05 01:07:16 PST
It's likely a hold-over from KDE/Qt, d was their pattern.
Hajime Morrita
Comment 2 2012-01-05 01:19:24 PST
Created attachment 121238 [details] Patch
Eric Seidel (no email)
Comment 3 2012-01-05 01:32:08 PST
Comment on attachment 121238 [details] Patch I assume m_internal is platform specific? It's unclear why all of this logic is on a separate object (and we're reaching into through double m_ dereferences.
Hajime Morrita
Comment 4 2012-01-05 02:24:32 PST
> I assume m_internal is platform specific? It's unclear why all of this logic is on a separate object (and we're reaching into through double m_ dereferences. No, basically it's just something like a pimpl pattern. All states are pushed into the internal object.
Gyuyoung Kim
Comment 5 2012-01-05 02:35:32 PST
Comment on attachment 121238 [details] Patch Attachment 121238 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11120201
Hajime Morrita
Comment 6 2012-01-05 02:47:56 PST
Created attachment 121248 [details] Patch
Gyuyoung Kim
Comment 7 2012-01-05 04:13:26 PST
Comment on attachment 121248 [details] Patch Attachment 121248 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/11110299
Early Warning System Bot
Comment 8 2012-01-05 05:02:35 PST
Comment on attachment 121248 [details] Patch Attachment 121248 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/11108306
Gustavo Noronha (kov)
Comment 9 2012-01-05 05:08:32 PST
Comment on attachment 121248 [details] Patch Attachment 121248 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/11131302
Eric Seidel (no email)
Comment 10 2012-01-05 10:36:06 PST
(In reply to comment #4) > > I assume m_internal is platform specific? It's unclear why all of this logic is on a separate object (and we're reaching into through double m_ dereferences. > No, basically it's just something like a pimpl pattern. > All states are pushed into the internal object. Since we no longer need pimpls (we have no c++ api), if the Internal object is not platform specific, a better fix would be to just collapse the two objects into one (instead of just fixing the style). Obviously that's a bit more involved. :)
WebKit Review Bot
Comment 11 2012-01-05 11:34:03 PST
Comment on attachment 121248 [details] Patch Attachment 121248 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/11147046
Hajime Morrita
Comment 12 2012-01-05 17:51:30 PST
Comment on attachment 121248 [details] Patch That makes sense. Removing r? for now.
Darin Adler
Comment 13 2012-01-08 18:46:09 PST
(In reply to comment #10) > (In reply to comment #4) > > > I assume m_internal is platform specific? It's unclear why all of this logic is on a separate object (and we're reaching into through double m_ dereferences. > > No, basically it's just something like a pimpl pattern. > > All states are pushed into the internal object. > > Since we no longer need pimpls (we have no c++ api), if the Internal object is not platform specific, a better fix would be to just collapse the two objects into one (instead of just fixing the style). Obviously that's a bit more involved. :) But pimpls are useful for more than just C++ API. They trade off additional memory use and slowness at runtime for less recompiling at compile time.
Note You need to log in before you can comment on or make changes to this bug.