New issue
Advanced search Search tips

Issue 788960 link

Starred by 1 user
Status: Fixed
Owner:
joelhockey@chromium.org
Closed: Dec 2017
Cc:
esprehn@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task



Sign in to add a comment

Mojofy/Onion Soup WebColorchooser

Project Member Reported by slangley@chromium.org, Nov 28 2017 
Replace the following IPCs with mojo (probably in their own mojom).

- FrameMsg_DidChooseColorResponse
- FrameMsg_DidEndColorChooser
- FrameHostMsg_OpenColorChooser

Onion soup renderer_webcolorchooser_impl.cc and remove any now redundant abstractions.

Comment 1 by slangley@chromium.org, Nov 28 2017 

BTW please check that slicing these IPCs into a separate mojo interface will not cause some ordering problem.

Comment 2 by joelhockey@chromium.org, Nov 28 2017 

I'm guessing we also want to include:

FrameHostMsg_SetSelectedColorInColorChooser

But probably not:

FrameHostMsg_DidChangeThemeColor
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 11 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/85b379dae71f793833f3e57bdbde5cc62dbc121b

commit 85b379dae71f793833f3e57bdbde5cc62dbc121b
Author: Joel Hockey <joelhockey@chromium.org>
Date: Mon Dec 11 10:42:13 2017

Convert ColorChooser from ipc to mojo.

Using a Factory to provide dedicated pipes for ColorChooser
and ColorChooserClient.  This removes the need for the identifier
currently in use for ipcs.  ColorChooserEnd messages
have been replaced with connection error handlers, and
each side can simply close the connection without explicitly
sending EndColorChooser msgs.

Replaced struct content::ColorSuggestion with
content::mojom::ColorSuggestion in content/public/common.


Bug:  788960 
Change-Id: I436e277738bf0e37eda8bfa0fd7428aede015c43
Reviewed-on: https://chromium-review.googlesource.com/800350
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: Nicholas Verne <nverne@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523071}
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/extensions/extension_view_host.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/extensions/extension_view_host.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/ui/app_list/custom_launcher_page_contents.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/ui/app_list/custom_launcher_page_contents.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/ui/browser.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/chrome/browser/ui/browser.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/components/guest_view/browser/guest_view_base.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/components/web_contents_delegate_android/color_chooser_android.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/components/web_contents_delegate_android/color_chooser_android.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/components/web_contents_delegate_android/web_contents_delegate_android.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/components/web_contents_delegate_android/web_contents_delegate_android.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/common/BUILD.gn
[add] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/common/color_chooser.mojom
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/common/frame_messages.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/public/common/BUILD.gn
[delete] https://crrev.com/47735b427fb7248d51c95d522291aa5a1ea99461/content/public/common/color_suggestion.cc
[delete] https://crrev.com/47735b427fb7248d51c95d522291aa5a1ea99461/content/public/common/color_suggestion.h
[add] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/public/common/color_suggestion.mojom
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/renderer/renderer_webcolorchooser_impl.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/content/renderer/renderer_webcolorchooser_impl.h
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/85b379dae71f793833f3e57bdbde5cc62dbc121b/extensions/browser/app_window/app_window.h
Project Member

Comment 4 by bugdroid1@chromium.org, Dec 20 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/163835a2375d6b7d67571e563fcaab39a7a79c6e

commit 163835a2375d6b7d67571e563fcaab39a7a79c6e
Author: Joel Hockey <joelhockey@chromium.org>
Date: Wed Dec 20 11:51:57 2017

Onion Soup ColorChooser

* Moved mojom definitions from content(/public)/common to
  WebKit/common/color_chooser.

* Mojo calls are now made directly from blink in ColorChooserUIController.

* Deleted unneeded implementation files:
 - content/renderer/renderer_webcolorchooser_impl.[h|cc]
 - third_party/WebKit/public/web/WebColorChooser.h
 - third_party/WebKit/public/web/WebColorChooserClient.h
 - third_party/WebKit/public/web/WebColorSuggestion.h
 - third_party/WebKit/Source/core/exported/WebColorSuggestion.cpp
 - third_party/WebKit/Source/platform/ColorSuggestion.h

* Deleted unneeded test mock files:
 - content/shell/test_runner/mock_color_chooser.[h|cc]
 - replaced with mock-colorchooser.js to intercept mojo calls.

Bug:  788960 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_dbg_ng
Change-Id: I305a2abb3cba457042c877e6401abbe383611c75
Reviewed-on: https://chromium-review.googlesource.com/821954
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525311}
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/build/check_gn_headers_whitelist.txt
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/chrome/browser/devtools/devtools_window.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/chrome/browser/devtools/devtools_window.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/chrome/browser/extensions/extension_view_host.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/chrome/browser/extensions/extension_view_host.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/chrome/browser/ui/browser.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/chrome/browser/ui/browser.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/guest_view/browser/guest_view_base.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/guest_view/browser/guest_view_base.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/web_contents_delegate_android/DEPS
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/web_contents_delegate_android/color_chooser_android.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/web_contents_delegate_android/color_chooser_android.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/web_contents_delegate_android/web_contents_delegate_android.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/components/web_contents_delegate_android/web_contents_delegate_android.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/common/BUILD.gn
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/public/app/mojo/content_browser_manifest.json
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/public/browser/web_contents_delegate.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/public/browser/web_contents_delegate.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/public/common/BUILD.gn
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/content/public/common/color_suggestion.mojom
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/renderer/BUILD.gn
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/renderer/render_frame_impl.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/renderer/render_view_impl.cc
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/content/renderer/renderer_webcolorchooser_impl.cc
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/content/renderer/renderer_webcolorchooser_impl.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/shell/test_runner/BUILD.gn
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/content/shell/test_runner/mock_color_chooser.cc
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/content/shell/test_runner/mock_color_chooser.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/shell/test_runner/test_runner.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/shell/test_runner/test_runner.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/shell/test_runner/web_frame_test_client.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/content/shell/test_runner/web_frame_test_proxy.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/extensions/browser/app_window/app_window.cc
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/extensions/browser/app_window/app_window.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/dom/shadow/input-color-in-content-expected.txt
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/dom/shadow/input-color-in-content.html
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/forms/color/display-none-input-color-chooser-shown.html
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/forms/color/input-color-chooser-shown-expected.txt
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/forms/color/input-color-chooser-shown-readonly-expected.txt
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/forms/color/input-color-chooser-shown-readonly.html
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/forms/color/input-color-chooser-shown.html
[add] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/LayoutTests/fast/forms/color/mock-colorchooser.js
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/exported/BUILD.gn
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/third_party/WebKit/Source/core/exported/WebColorSuggestion.cpp
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorChooserClient.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorChooserPopupUIController.cpp
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorChooserPopupUIController.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorChooserUIController.cpp
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorChooserUIController.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorInputType.cpp
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/ColorInputType.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/html/forms/InternalPopupMenu.cpp
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/page/ChromeClientImpl.cpp
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/Source/core/page/ChromeClientImplTest.cpp
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/third_party/WebKit/Source/platform/ColorSuggestion.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/common/BUILD.gn
[add] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/common/color_chooser/OWNERS
[rename] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/common/color_chooser/color_chooser.mojom
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/public/BUILD.gn
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/third_party/WebKit/public/web/WebColorChooser.h
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/third_party/WebKit/public/web/WebColorChooserClient.h
[delete] https://crrev.com/479f16864adf206a648225669377ba7bdaafcd49/third_party/WebKit/public/web/WebColorSuggestion.h
[modify] https://crrev.com/163835a2375d6b7d67571e563fcaab39a7a79c6e/third_party/WebKit/public/web/WebFrameClient.h

Comment 5 by joelhockey@chromium.org, Dec 20 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment