Bug 119025 - Rename WebColorChooserProxy
Summary: Rename WebColorChooserProxy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks: 61276
  Show dependency treegraph
 
Reported: 2013-07-23 16:52 PDT by Ruth Fong
Modified: 2013-07-24 18:07 PDT (History)
19 users (show)

See Also:


Attachments
Patch (28.65 KB, patch)
2013-07-24 14:08 PDT, Ruth Fong
no flags Details | Formatted Diff | Diff
Patch (54.79 KB, patch)
2013-07-24 14:52 PDT, Ruth Fong
no flags Details | Formatted Diff | Diff
Patch (54.48 KB, patch)
2013-07-24 16:07 PDT, Ruth Fong
no flags Details | Formatted Diff | Diff
Patch (54.44 KB, patch)
2013-07-24 17:02 PDT, Ruth Fong
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ruth Fong 2013-07-23 16:52:09 PDT
WebColorChooserProxy, which represents the color picker UI for an <input type='color'> element, suggests a one-to-one relationship with the WebColorChooser object. 

In bug 61276, the architecture will change so that only one  WebColorChooserProxy object exists for a given WebPageProxy, while there can be multiple WebColorChooser objects living in the WebProcess.

WebColorChooserProxy should be renamed to something like WebColorPickerUI that reflects its representation of the color picker UI.
Comment 1 Radar WebKit Bug Importer 2013-07-23 16:53:07 PDT
<rdar://problem/14528039>
Comment 2 Ruth Fong 2013-07-24 14:08:22 PDT
Created attachment 207410 [details]
Patch
Comment 3 Early Warning System Bot 2013-07-24 14:12:56 PDT
Comment on attachment 207410 [details]
Patch

Attachment 207410 [details] did not pass qt-wk2-ews (qt-wk2):
Output: http://webkit-queues.appspot.com/results/1200027
Comment 4 EFL EWS Bot 2013-07-24 14:15:04 PDT
Comment on attachment 207410 [details]
Patch

Attachment 207410 [details] did not pass efl-wk2-ews (efl-wk2):
Output: http://webkit-queues.appspot.com/results/1205034
Comment 5 Brady Eidson 2013-07-24 14:16:18 PDT
Comment on attachment 207410 [details]
Patch

Should change it in all the build systems.

`grep -r WebColorChooserProxy.cpp <path/to/WebKit2>` should find everywhere you need to replace it.
Comment 6 Tim Horton 2013-07-24 14:17:11 PDT
Comment on attachment 207410 [details]
Patch

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

> Source/WebKit2/UIProcess/PageClient.h:190
> +    virtual PassRefPtr<WebColorPickerUI> createColorChooserProxy(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) = 0;

This method is poorly named now, isn't it?
Comment 7 Brady Eidson 2013-07-24 14:25:06 PDT
(In reply to comment #6)
> (From update of attachment 207410 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=207410&action=review
> 
> > Source/WebKit2/UIProcess/PageClient.h:190
> > +    virtual PassRefPtr<WebColorPickerUI> createColorChooserProxy(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) = 0;
> 
> This method is poorly named now, isn't it?


And is there any advantage to calling the new object WebColorPickerUI instead of WebColorPicker, considering that the abstract concept of a WebColorPicker encapsulates more than just its UI?
Comment 8 Ruth Fong 2013-07-24 14:42:05 PDT
Comment on attachment 207410 [details]
Patch

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

>>> Source/WebKit2/UIProcess/PageClient.h:190
>>> +    virtual PassRefPtr<WebColorPickerUI> createColorChooserProxy(WebPageProxy*, const WebCore::Color& initialColor, const WebCore::IntRect&) = 0;
>> 
>> This method is poorly named now, isn't it?
> 
> And is there any advantage to calling the new object WebColorPickerUI instead of WebColorPicker, considering that the abstract concept of a WebColorPicker encapsulates more than just its UI?

I think that the WebColorPicker(UI) object does represent UI representation of the color picker, whereas the WebColorChooser is the abstract representation for the <input type='color'> object.
Comment 9 Ruth Fong 2013-07-24 14:52:56 PDT
Created attachment 207412 [details]
Patch
Comment 10 WebKit Commit Bot 2013-07-24 14:56:06 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 11 Ruth Fong 2013-07-24 16:07:03 PDT
Created attachment 207417 [details]
Patch
Comment 12 Brady Eidson 2013-07-24 16:55:55 PDT
Comment on attachment 207417 [details]
Patch

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

> Source/WebKit2/ChangeLog:10
> +        Rename WebColorChooserProxy to WebColorPicker since the name WebColorChoooserProxy
> +        implies a one-to-one relationship with WebColorChooser, which will not be true when
> +        the patch for bug 61276 lands.

Nitpick - It's not even true *now*

Yes, the WebProcess has 1 WebColorChooser, and the UIProcess has 1 WebColorChooserProxy...  but they aren't actually connected.

The Object/ObjectProxy naming scheme is for a pair of objects that represent the identical object/concept on both sides of the IPC connection, so tightly intertwined that they message each other directly.
Comment 13 Ruth Fong 2013-07-24 17:02:10 PDT
Created attachment 207422 [details]
Patch
Comment 14 WebKit Commit Bot 2013-07-24 17:03:46 PDT
Comment on attachment 207422 [details]
Patch

Rejecting attachment 207422 [details] from commit-queue.

ruthiecftg@gmail.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 15 WebKit Commit Bot 2013-07-24 18:07:17 PDT
Comment on attachment 207422 [details]
Patch

Clearing flags on attachment: 207422

Committed r153108: <http://trac.webkit.org/changeset/153108>
Comment 16 WebKit Commit Bot 2013-07-24 18:07:23 PDT
All reviewed patches have been landed.  Closing bug.