Closed Bug 963294 Opened 6 years ago Closed 6 years ago

[e10s] Implement a proxy for the color picker

Categories

(Toolkit :: General, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla30

People

(Reporter: evilpie, Assigned: evilpie)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch color-picker (obsolete) — Splinter Review
So I decided, because bug 910384 is blocked on that FileHandle stuff to instead implement <input type=color>. We have a lot of the same life-time issues as the other case, which I need help figuring out. Inheriting from both nsIColorPicker and PColorPickerChild seems to be hard. It seems like we should deref nsColorPickerProxy when IPDL isn't going to touch it again. Not sure what the right point for that is though: Proxy::Recv__delete__ or TabChild::DeallocPColorPickerChild (this looks kinda good?)
Attachment #8364656 - Attachment is patch: true
Comment on attachment 8364656 [details] [diff] [review]
color-picker

Thanks, I'll take a look soon. I've been thinking of how we can generalize some of the nsWidgetFactory stuff, so maybe we can work this in.
Attachment #8364656 - Flags: feedback?(wmccloskey)
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attached patch color-picker v2 (obsolete) — Splinter Review
Although probably violating all kinds of best practices, I think I figured out the lifetime management in the child.
Attachment #8364656 - Attachment is obsolete: true
Attachment #8364656 - Flags: feedback?(wmccloskey)
No longer blocks: 910384
Attached patch color-picker v3Splinter Review
Attachment #8365515 - Attachment is obsolete: true
Attachment #8377819 - Flags: review?(josh)
Comment on attachment 8377819 [details] [diff] [review]
color-picker v3

Review of attachment 8377819 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/ipc/ColorPickerParent.cpp
@@ +43,5 @@
> +  mColorPickerParent = nullptr;
> +}
> +
> +bool
> +ColorPickerParent::CreatColorPicker()

Create

::: dom/ipc/PBrowser.ipdl
@@ +222,5 @@
>       */
>      ShowTooltip(uint32_t x, uint32_t y, nsString tooltip);
>      HideTooltip();
>  
> +    PColorPicker(nsString title, nsString initialColor);

Can't hurt to document this.

::: dom/ipc/moz.build
@@ +17,5 @@
>      'nsIRemoteBlob.h',
>  ]
>  
>  EXPORTS.mozilla.dom += [
> +    'ColorPickerParent.h',

Does this need to be exported?

::: widget/xpwidgets/nsColorPickerProxy.h
@@ +3,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef nsColorPickerProxy_h__
> +#define nsColorPickerProxy_h__

Scrap the __ on these lines.

@@ +17,5 @@
> +  NS_DECL_ISUPPORTS
> +  NS_DECL_NSICOLORPICKER
> +
> +  nsColorPickerProxy() {};
> +  ~nsColorPickerProxy() {};

No need for ; on these lines
Attachment #8377819 - Flags: review?(josh) → review+
https://hg.mozilla.org/mozilla-central/rev/02959a932fb7
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.