Issue 1423263004: [MD] Use programmatic rendering for omnibox chips

Issue 1423263004: [MD] Use programmatic rendering for omnibox chips (Closed)

Created:
5 years, 1 month ago by Evan Stade

Modified:
5 years, 1 month ago

Reviewers:
Peter Kasting, jonross, oshima

CC:
chromium-reviews, tfarina, oshima+watch_chromium.org

Base URL:
https://chromium.googlesource.com/chromium/src.git@master

Target Ref:
refs/pending/heads/master

Project:
chromium

Visibility:
Public.

More Reviews

Description

[MD] Use programmatic rendering for omnibox chips BUG=551729 Committed: https://crrev.com/e9ac857be7ea567accb58312237ca31b5b559f37 Cr-Commit-Position: refs/heads/master@{#358452}

Patch Set 3 : pkasting review #

Total comments: 1

Patch Set 4 : move fns a little #

Patch Set 5 : rebase #

Created: 5 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+82 lines, -63 lines)			Patch
D	chrome/app/theme/default_100_percent/common/omnibox_content_setting_bubble.png	View		Binary file		0 comments	Download
D	chrome/app/theme/default_100_percent/common/omnibox_ev_bubble.png	View		Binary file		0 comments	Download
D	chrome/app/theme/default_100_percent/common/omnibox_selected_keyword_bubble.png	View		Binary file		0 comments	Download
D	chrome/app/theme/default_200_percent/common/omnibox_content_setting_bubble.png	View		Binary file		0 comments	Download
D	chrome/app/theme/default_200_percent/common/omnibox_ev_bubble.png	View		Binary file		0 comments	Download
D	chrome/app/theme/default_200_percent/common/omnibox_selected_keyword_bubble.png	View		Binary file		0 comments	Download
M	chrome/app/theme/theme_resources.grd	View	1 2 3 4	3 chunks	+0 lines, -7 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/content_setting_image_view.h	View	1 2	2 chunks	+4 lines, -1 line	0 comments	Download
M	chrome/browser/ui/views/location_bar/content_setting_image_view.cc	View	1 2	3 chunks	+11 lines, -7 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/ev_bubble_view.h	View	1 2	2 chunks	+8 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/ev_bubble_view.cc	View	1 2	2 chunks	+10 lines, -6 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/icon_label_bubble_view.h	View	1 2 3	5 chunks	+10 lines, -8 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/icon_label_bubble_view.cc	View		5 chunks	+18 lines, -11 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/location_bar_view.h	View	1 2 3 4	1 chunk	+0 lines, -1 line	0 comments	Download
M	chrome/browser/ui/views/location_bar/location_bar_view.cc	View	1 2 3 4	5 chunks	+6 lines, -12 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/selected_keyword_view.h	View	1 2	1 chunk	+4 lines, -0 lines	0 comments	Download
M	chrome/browser/ui/views/location_bar/selected_keyword_view.cc	View	1 2	2 chunks	+11 lines, -10 lines	0 comments	Download

Messages

Total messages: 32 (15 generated)

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages

Evan Stade

this was surprisingly painless. The only thing that's slightly different from the raster assets is ...

5 years, 1 month ago (2015-11-05 03:14:19 UTC) #3

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423263004/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423263004/20001

5 years, 1 month ago (2015-11-05 03:14:35 UTC) #5

commit-bot: I haz the power

Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_compile_dbg_ng/builds/9784) mac_chromium_gn_rel on ...

5 years, 1 month ago (2015-11-05 03:21:46 UTC) #7

Peter Kasting

Nice. LGTM https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/content_setting_image_view.h File chrome/browser/ui/views/location_bar/content_setting_image_view.h (right): https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/content_setting_image_view.h#newcode64 chrome/browser/ui/views/location_bar/content_setting_image_view.h:64: SkColor GetBorderColor() const override; Nit: List overrides ...

5 years, 1 month ago (2015-11-05 06:22:11 UTC) #8

jonross

On 2015/11/05 06:22:11, Peter Kasting wrote: > Nice. LGTM > > https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/content_setting_image_view.h > File chrome/browser/ui/views/location_bar/content_setting_image_view.h ...

5 years, 1 month ago (2015-11-05 14:46:21 UTC) #9

Evan Stade

https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/content_setting_image_view.h File chrome/browser/ui/views/location_bar/content_setting_image_view.h (right): https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/content_setting_image_view.h#newcode64 chrome/browser/ui/views/location_bar/content_setting_image_view.h:64: SkColor GetBorderColor() const override; On 2015/11/05 06:22:11, Peter Kasting ...

5 years, 1 month ago (2015-11-05 23:59:06 UTC) #10

Evan Stade

https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/location_bar_view.cc File chrome/browser/ui/views/location_bar/location_bar_view.cc (right): https://codereview.chromium.org/1423263004/diff/20001/chrome/browser/ui/views/location_bar/location_bar_view.cc#newcode370 chrome/browser/ui/views/location_bar/location_bar_view.cc:370: color = SkColorSetRGB(7, 149, 0); On 2015/11/05 06:22:11, Peter ...

5 years, 1 month ago (2015-11-05 23:59:29 UTC) #11

commit-bot: I haz the power

Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423263004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423263004/40001

5 years, 1 month ago (2015-11-06 00:05:22 UTC) #13

commit-bot: I haz the power

Dry run: This issue passed the CQ dry run.

5 years, 1 month ago (2015-11-06 00:52:25 UTC) #15

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423263004/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423263004/40001

5 years, 1 month ago (2015-11-06 02:04:19 UTC) #18

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423263004/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423263004/60001

5 years, 1 month ago (2015-11-06 18:52:38 UTC) #25

commit-bot: I haz the power

Failed to apply patch for chrome/app/theme/theme_resources.grd: While running git apply --index -3 -p1; error: patch ...

5 years, 1 month ago (2015-11-06 21:00:03 UTC) #27

Failed to apply patch for chrome/app/theme/theme_resources.grd:
While running git apply --index -3 -p1;
  error: patch failed: chrome/app/theme/theme_resources.grd:498
  Falling back to three-way merge...
  Applied patch to 'chrome/app/theme/theme_resources.grd' with conflicts.
  U chrome/app/theme/theme_resources.grd

Patch:       chrome/app/theme/theme_resources.grd
Index: chrome/app/theme/theme_resources.grd
diff --git a/chrome/app/theme/theme_resources.grd
b/chrome/app/theme/theme_resources.grd
index
9191333729ef06043eece0444f710b25aa0cdd41..a8c2b48107917d4ec4e5e1a42d22413b9900d521
100644
--- a/chrome/app/theme/theme_resources.grd
+++ b/chrome/app/theme/theme_resources.grd
@@ -483,9 +483,6 @@
       <structure type="chrome_scaled_image" name="IDR_NOTIFIER_BLOCK_BUTTON"
file="common/block_notifier.png" />
       <structure type="chrome_scaled_image" name="IDR_OMNIBOX_BORDER"
file="common/omnibox_border.png" />
       <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_CALCULATOR_SELECTED"
file="common/omnibox_calculator_selected.png" />
-      <if expr="enable_topchrome_md">
-        <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_CONTENT_SETTING_BUBBLE"
file="common/omnibox_content_setting_bubble.png" />
-      </if>
       <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_CONTENT_SETTING_BUBBLE_BOTTOM"
file="common/omnibox_content_setting_bubble_bottom.png" />
       <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_CONTENT_SETTING_BUBBLE_BOTTOM_LEFT"
file="common/omnibox_content_setting_bubble_bottom_left.png" />
       <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_CONTENT_SETTING_BUBBLE_BOTTOM_RIGHT"
file="common/omnibox_content_setting_bubble_bottom_right.png" />
@@ -498,7 +495,6 @@
       <if expr="enable_topchrome_md">
         <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_DROPDOWN_SHADOW_BOTTOM"
file="common/omnibox_dropdown_shadow_bottom.png" />
         <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_DROPDOWN_SHADOW_TOP"
file="common/omnibox_dropdown_shadow_top.png" />
-        <structure type="chrome_scaled_image" name="IDR_OMNIBOX_EV_BUBBLE"
file="common/omnibox_ev_bubble.png" />
         <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_EV_BUBBLE_HTTPS_VALID"
file="common/omnibox_ev_bubble_https_valid.png" />
       </if>
       <structure type="chrome_scaled_image" name="IDR_OMNIBOX_EV_BUBBLE_BOTTOM"
file="common/omnibox_ev_bubble_bottom.png" />
@@ -536,7 +532,6 @@
       <structure type="chrome_scaled_image" name="IDR_OMNIBOX_SEARCH_SECURED"
file="common/omnibox_search_secured.png" />
       <structure type="chrome_scaled_image" name="IDR_OMNIBOX_SEARCH_SELECTED"
file="common/omnibox_search_selected.png" />
       <if expr="enable_topchrome_md">
-        <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_SELECTED_KEYWORD_BUBBLE"
file="common/omnibox_selected_keyword_bubble.png" />
         <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_SELECTED_KEYWORD_BUBBLE_SEARCH"
file="common/omnibox_selected_keyword_bubble_search.png" />
       </if>
       <structure type="chrome_scaled_image"
name="IDR_OMNIBOX_SELECTED_KEYWORD_BUBBLE_BOTTOM"
file="common/omnibox_selected_keyword_bubble_bottom.png" />

commit-bot: I haz the power

CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1423263004/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1423263004/80001

5 years, 1 month ago (2015-11-06 23:14:35 UTC) #30

commit-bot: I haz the power

5 years, 1 month ago (2015-11-06 23:56:10 UTC) #32

Message was sent while issue was closed.

Patchset 5 (id:??) landed as
https://crrev.com/e9ac857be7ea567accb58312237ca31b5b559f37
Cr-Commit-Position: refs/heads/master@{#358452}

Expand Messages | Collapse Messages | Show Generated Messages | Hide Generated Messages