Implement input type=color UI

chrome/browser/ui/browser.cc

Index: chrome/browser/ui/browser.cc
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 8c71cbf8becf8aeea41f4564b3d6e8673d457dd4..5edf07be2bff3899c565b019a0179a34cb593e28 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -106,6 +106,7 @@
 #include "chrome/browser/ui/browser_navigator.h"
 #include "chrome/browser/ui/browser_tab_restore_service_delegate.h"
 #include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/color_chooser.h"
 #include "chrome/browser/ui/constrained_window_tab_helper.h"
 #include "chrome/browser/ui/extensions/shell_window.h"
 #include "chrome/browser/ui/find_bar/find_bar.h"
@@ -4021,6 +4022,44 @@ content::JavaScriptDialogCreator* Browser::GetJavaScriptDialogCreator() {
   return GetJavaScriptDialogCreatorInstance();
 }
 
+void Browser::OpenColorChooser(WebContents* tab,
+                               unsigned color_chooser_id,
+                               const SkColor& color) {
+  printf("! Browser::OpenColorChooser %p %d\n", this, color_chooser_id);

[Peter Kasting, 2012/02/07 01:46:24]

No printfs (3 places)

[keishi, 2012/02/17 11:31:05]

Done.

+  if (color_chooser_.get() && color_chooser_id == color_chooser_->identifier())
+    return;
+  if (color_chooser_.get())
+    color_chooser_->End();
+  color_chooser_.reset(ColorChooser::Create(color_chooser_id,
+                                            tab->GetRenderViewHost()));
+  color_chooser_->Open(color);
+}
+void Browser::EndColorChooser(WebContents* tab, unsigned color_chooser_id) {

[Peter Kasting, 2012/02/07 01:46:24]

In general I don't understand why you check both the RVH pointers for equality
AND have the ID checks.  It seems like one must be a superset of the other?  Or
is that not true?

[keishi, 2012/02/17 11:31:05]

The color chooser id is generated by incrementing a counter inside the renderer
process so it overlaps for different renderers. 

+  printf("! Browser::EndColorChooser %p\n", this);
+  if (tab->GetRenderViewHost() != color_chooser_->render_view_host())
+    return;
+  if (tab->GetRenderViewHost() != color_chooser_->render_view_host())

[Peter Kasting, 2012/02/07 01:46:24]

Nit: Two copies of this line, I think you meant to check the ID

[keishi, 2012/02/17 11:31:05]

Done.

+    return;
+  color_chooser_->End();
+}
+void Browser::SetSelectedColorInColorChooser(WebContents* tab,
+    unsigned color_chooser_id,

[Peter Kasting, 2012/02/07 01:46:24]

Nit: Indent even.  If it won't fit, indent even the first arg 4

[keishi, 2012/02/17 11:31:05]

Done.

+    const SkColor& color) {
+  if (!color_chooser_.get())

[Peter Kasting, 2012/02/07 01:46:24]

Nit: Combine these conditionals

[keishi, 2012/02/17 11:31:05]

Done.

+    return;
+  if (tab->GetRenderViewHost() != color_chooser_->render_view_host())
+    return;
+  if (color_chooser_id != color_chooser_->identifier())
+    return;
+  color_chooser_->SetSelectedColor(color);
+}
+void Browser::DidEndColorChooser(WebContents* tab) {
+  printf("! Browser::DidEndColorChooser %p\n", this);
+  if (tab->GetRenderViewHost() != color_chooser_->render_view_host())
+    return;
+  color_chooser_.reset();
+}
+
 void Browser::RunFileChooser(WebContents* tab,
                              const content::FileChooserParams& params) {
   RunFileChooserHelper(tab, params);