[Webkit-unassigned] [Bug 96894] Add DeviceController base-class to remove duplication of DeviceXXXControler

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=96894





--- Comment #6 from Kihong Kwon <kihong.kwon at samsung.com>  2012-09-20 03:18:13 PST ---
(In reply to comment #4)
> (From update of attachment 164340 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=164340&action=review
> 
> I think the root source of (seemingly unnecessary) complexity is the responsibility assignment between client and the controller.
> It smells possible wrong design if we see client creating an event object.
> 
> Can we eliminate preservedEvent() and hasPreservedEvent() by moving the logic to the controller?
> That would simplify the interaction between controller and client. 
> Since there is no client implementation available for now, it's hard to see whether this complexity is really necessary.
> And I expect it isn't. 
> 
> What do you think?
It's better. I will change this like yours.

> 
> > Source/WebCore/page/DeviceController.cpp:39
> > +    int isEmpty = m_listeners.size();
> 
> What does this mean?
> - Why is "isEmpty" int not bool?
> - Why non-empty listener vector implies isEmpty?
I will change to bool and use isEmpty() of Set.
And I think wasEmpty is more reasonable name in this case.

> 
> > Source/WebCore/page/DeviceController.cpp:41
> > +    if (m_listeners.find(window) == m_listeners.end())
> 
> You can just add it. That's how set works.
You're right. I need to change this.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.