Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(373)

Issue 1130033003: Make the minimize state work when creating a chrome window in CrOS (Closed)

Created:
2 years, 7 months ago by joone
Modified:
1 year, 8 months ago
CC:
chromium-reviews, tfarina, tdanderson+views_chromium.org, not at google - send to devlin
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make the minimize state work when creating a chrome window in CrOS This CL makes SHOW_STATE_MINIMIZE state work when creating a chrome window in CrOS, so we don't need to call Minimize() manually in tabs_api.cc BUG=473228 TEST=NativeWidgetAuraTest.CreateMinimized Committed: https://crrev.com/d5d6762c5539bc7d6098588323a16a24826e1149 Cr-Commit-Position: refs/heads/master@{#361898}

Patch Set 1 #

Total comments: 2

Patch Set 2 : Removed switch statement #

Total comments: 4

Patch Set 3 : Pass the test case without CrOS condition #

Total comments: 1

Patch Set 4 : Make window minimize #

Total comments: 2

Patch Set 5 : rebase on master #

Total comments: 3

Patch Set 6 : Add Minimized() call in `if (state !=ui::SHOW_STATE_INACTIVE)` condition #

Total comments: 4

Patch Set 7 : call SetProperty() after Show() is called #

Patch Set 8 : a patch for testing the launcher bug #

Total comments: 1

Patch Set 9 : Minimize at the end #

Total comments: 1

Patch Set 10 : Add comment #

Patch Set 11 : Add test case #

Total comments: 2

Patch Set 12 : Updated test case #

Total comments: 2

Patch Set 13 : s/MinimizedStateTest/CreateMinimized #

Total comments: 2

Patch Set 14 : Add widget->Show(), widget->Minimize() calls #

Total comments: 3

Patch Set 15 : ui::SHOW_STATE_MINIMIZED was considered in Widget::Show() #

Patch Set 16 : rebase on master #

Total comments: 2

Patch Set 17 : fix test failures #

Total comments: 4

Patch Set 18 : rebase on master #

Total comments: 2

Patch Set 19 : apply the new condition code #

Total comments: 2

Patch Set 20 : remove the parentheses in the inline if statement #

Unified diffs Side-by-side diffs Delta from patch set Stats (+24 lines, -9 lines) Patch
M chrome/browser/extensions/api/tabs/tabs_api.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +0 lines, -7 lines 0 comments Download
M ui/views/widget/native_widget_aura.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +5 lines, -0 lines 0 comments Download
M ui/views/widget/native_widget_aura_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +15 lines, -0 lines 0 comments Download
M ui/views/widget/widget.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 114 (20 generated)
joone
sadrul@ could you review this patch?
2 years, 7 months ago (2015-05-15 20:15:00 UTC) #2
sadrul
+oshima@ https://codereview.chromium.org/1130033003/diff/1/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/1/ui/views/widget/native_widget_aura.cc#newcode503 ui/views/widget/native_widget_aura.cc:503: Looks like you can just do window_->SetProperty() unconditionally, ...
2 years, 7 months ago (2015-05-15 20:30:54 UTC) #5
oshima
https://codereview.chromium.org/1130033003/diff/1/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/1/ui/views/widget/native_widget_aura.cc#newcode503 ui/views/widget/native_widget_aura.cc:503: On 2015/05/15 20:30:54, sadrul wrote: > Looks like you ...
2 years, 7 months ago (2015-05-15 20:45:53 UTC) #6
joone
On 2015/05/15 20:45:53, oshima wrote: > https://codereview.chromium.org/1130033003/diff/1/ui/views/widget/native_widget_aura.cc > File ui/views/widget/native_widget_aura.cc (right): > > https://codereview.chromium.org/1130033003/diff/1/ui/views/widget/native_widget_aura.cc#newcode503 > ...
2 years, 7 months ago (2015-05-18 18:16:34 UTC) #7
sadrul
https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc#newcode490 ui/views/widget/native_widget_aura.cc:490: Why? This looks like a change in behaviour.
2 years, 7 months ago (2015-05-18 19:51:43 UTC) #8
joone
https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc#newcode490 ui/views/widget/native_widget_aura.cc:490: On 2015/05/18 19:51:42, sadrul wrote: > Why? This looks ...
2 years, 7 months ago (2015-05-18 20:22:44 UTC) #9
sadrul
https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc#newcode490 ui/views/widget/native_widget_aura.cc:490: On 2015/05/18 20:22:43, joone wrote: > On 2015/05/18 19:51:42, ...
2 years, 7 months ago (2015-05-18 20:29:28 UTC) #10
joone
On 2015/05/18 20:29:28, sadrul wrote: > https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc > File ui/views/widget/native_widget_aura.cc (right): > > https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc#newcode490 > ...
2 years, 7 months ago (2015-05-18 20:40:51 UTC) #11
oshima
https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc#newcode490 ui/views/widget/native_widget_aura.cc:490: On 2015/05/18 20:29:27, sadrul wrote: > On 2015/05/18 20:22:43, ...
2 years, 7 months ago (2015-05-18 20:48:07 UTC) #12
joone
On 2015/05/18 20:48:07, oshima wrote: > https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc > File ui/views/widget/native_widget_aura.cc (right): > > https://codereview.chromium.org/1130033003/diff/20001/ui/views/widget/native_widget_aura.cc#newcode490 > ...
2 years, 7 months ago (2015-05-19 00:29:14 UTC) #13
oshima
can you double check with the owner if the decision was made based on the ...
2 years, 7 months ago (2015-05-19 22:53:00 UTC) #14
joone
On 2015/05/19 22:53:00, oshima wrote: > can you double check with the owner if the ...
2 years, 7 months ago (2015-05-19 23:23:28 UTC) #15
joone
On 2015/05/19 22:53:00, oshima wrote: > can you double check with the owner if the ...
2 years, 7 months ago (2015-05-19 23:24:52 UTC) #16
limasdf
Sorry to interrupt. Joone, you can find a test extension on crbug.com/459841 With this CL, ...
2 years, 7 months ago (2015-05-20 16:47:24 UTC) #18
not at google - send to devlin
There was a lot of investigation into why some platforms didn't support some of the ...
2 years, 7 months ago (2015-05-20 20:18:48 UTC) #20
joone
On 2015/05/20 16:47:24, Sung-guk Lim wrote: > Sorry to interrupt. > > Joone, you can ...
2 years, 7 months ago (2015-05-20 22:55:20 UTC) #21
joone
On 2015/05/20 20:18:48, kalman wrote: > There was a lot of investigation into why some ...
2 years, 7 months ago (2015-05-20 23:09:58 UTC) #22
joone
On 2015/05/20 23:09:58, joone wrote: > On 2015/05/20 20:18:48, kalman wrote: > > There was ...
2 years, 6 months ago (2015-05-26 17:30:44 UTC) #23
not at google - send to devlin
This looks fine, but @limasdf there was a test you added for this - is ...
2 years, 6 months ago (2015-05-26 20:11:26 UTC) #24
tapted
On 2015/05/26 20:11:26, kalman wrote: > This looks fine, but @limasdf there was a test ...
2 years, 6 months ago (2015-05-26 23:25:30 UTC) #25
joone
https://codereview.chromium.org/1130033003/diff/60001/chrome/browser/extensions/api/tabs/tabs_api.cc File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/1130033003/diff/60001/chrome/browser/extensions/api/tabs/tabs_api.cc#newcode666 chrome/browser/extensions/api/tabs/tabs_api.cc:666: // once mapped, but not minimize or fullscreen. On ...
2 years, 6 months ago (2015-05-27 17:50:22 UTC) #26
joone
On 2015/05/27 17:50:22, joone wrote: > https://codereview.chromium.org/1130033003/diff/60001/chrome/browser/extensions/api/tabs/tabs_api.cc > File chrome/browser/extensions/api/tabs/tabs_api.cc (right): > > https://codereview.chromium.org/1130033003/diff/60001/chrome/browser/extensions/api/tabs/tabs_api.cc#newcode666 > ...
2 years, 6 months ago (2015-05-27 21:57:43 UTC) #27
joone
tapted@, kalman@, thanks for the review on https://codereview.chromium.org/1159703006/ Due to the change, I rebased this ...
2 years, 6 months ago (2015-05-28 16:52:18 UTC) #28
tapted
https://codereview.chromium.org/1130033003/diff/80001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/80001/ui/views/widget/native_widget_aura.cc#newcode499 ui/views/widget/native_widget_aura.cc:499: Minimize(); Minimize() is literally `window_->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MINIMIZED);` so this should ...
2 years, 6 months ago (2015-05-28 23:55:02 UTC) #29
tapted
https://codereview.chromium.org/1130033003/diff/80001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/80001/ui/views/widget/native_widget_aura.cc#newcode499 ui/views/widget/native_widget_aura.cc:499: Minimize(); On 2015/05/28 23:55:02, tapted wrote: > Minimize() is ...
2 years, 6 months ago (2015-05-29 00:02:16 UTC) #30
joone
https://codereview.chromium.org/1130033003/diff/80001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/80001/ui/views/widget/native_widget_aura.cc#newcode499 ui/views/widget/native_widget_aura.cc:499: Minimize(); On 2015/05/29 00:02:16, tapted wrote: > On 2015/05/28 ...
2 years, 6 months ago (2015-05-29 00:52:01 UTC) #31
tapted
https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc#newcode492 ui/views/widget/native_widget_aura.cc:492: if (state == ui::SHOW_STATE_MINIMIZED) Sorry - I meant this ...
2 years, 6 months ago (2015-05-29 00:57:42 UTC) #32
joone
https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc#newcode492 ui/views/widget/native_widget_aura.cc:492: if (state == ui::SHOW_STATE_MINIMIZED) On 2015/05/29 00:57:42, tapted wrote: ...
2 years, 6 months ago (2015-05-29 01:11:27 UTC) #33
tapted
https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc#newcode492 ui/views/widget/native_widget_aura.cc:492: if (state == ui::SHOW_STATE_MINIMIZED) On 2015/05/29 01:11:27, joone wrote: ...
2 years, 6 months ago (2015-05-29 01:25:12 UTC) #34
oshima
https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/100001/ui/views/widget/native_widget_aura.cc#newcode492 ui/views/widget/native_widget_aura.cc:492: if (state == ui::SHOW_STATE_MINIMIZED) On 2015/05/29 01:25:12, tapted wrote: ...
2 years, 6 months ago (2015-05-29 09:24:21 UTC) #35
joone
oshima@ I tried to skip the show if the minimized state is requested, but a ...
2 years, 6 months ago (2015-05-29 18:34:13 UTC) #36
oshima
On 2015/05/29 18:34:13, joone wrote: > oshima@ I tried to skip the show if the ...
2 years, 6 months ago (2015-05-29 18:35:42 UTC) #37
joone
On 2015/05/29 18:35:42, oshima wrote: > On 2015/05/29 18:34:13, joone wrote: > > oshima@ I ...
2 years, 6 months ago (2015-05-29 19:47:49 UTC) #38
joone
On 2015/05/29 19:47:49, joone wrote: > On 2015/05/29 18:35:42, oshima wrote: > > On 2015/05/29 ...
2 years, 6 months ago (2015-05-29 19:50:40 UTC) #39
oshima
On 2015/05/29 19:50:40, joone wrote: > On 2015/05/29 19:47:49, joone wrote: > > On 2015/05/29 ...
2 years, 6 months ago (2015-05-29 19:58:30 UTC) #40
joone
On 2015/05/29 19:58:30, oshima wrote: > On 2015/05/29 19:50:40, joone wrote: > > On 2015/05/29 ...
2 years, 6 months ago (2015-05-29 19:59:45 UTC) #41
joone
oshima@ what do you think about the patch set 7?
2 years, 6 months ago (2015-06-01 22:42:54 UTC) #42
oshima
On 2015/06/01 22:42:54, joone wrote: > oshima@ what do you think about the patch set ...
2 years, 6 months ago (2015-06-01 22:50:22 UTC) #43
joone
On 2015/06/01 22:50:22, oshima wrote: > On 2015/06/01 22:42:54, joone wrote: > > oshima@ what ...
2 years, 6 months ago (2015-06-01 23:04:31 UTC) #44
oshima
On 2015/06/01 23:04:31, joone wrote: > On 2015/06/01 22:50:22, oshima wrote: > > On 2015/06/01 ...
2 years, 6 months ago (2015-06-01 23:08:46 UTC) #45
joone
On 2015/06/01 23:08:46, oshima wrote: > On 2015/06/01 23:04:31, joone wrote: > > On 2015/06/01 ...
2 years, 6 months ago (2015-06-02 23:59:14 UTC) #46
oshima
On 2015/06/02 23:59:14, joone wrote: > On 2015/06/01 23:08:46, oshima wrote: > > On 2015/06/01 ...
2 years, 6 months ago (2015-06-03 01:15:43 UTC) #47
joone
On 2015/06/03 01:15:43, oshima wrote: > On 2015/06/02 23:59:14, joone wrote: > > On 2015/06/01 ...
2 years, 6 months ago (2015-06-03 02:32:37 UTC) #48
oshima
https://codereview.chromium.org/1130033003/diff/140001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/140001/ui/views/widget/native_widget_aura.cc#newcode489 ui/views/widget/native_widget_aura.cc:489: window_->Show(); Thank you joone@ for investigation. It turns out ...
2 years, 6 months ago (2015-06-06 00:47:35 UTC) #49
joone
On 2015/06/06 00:47:35, oshima wrote: > https://codereview.chromium.org/1130033003/diff/140001/ui/views/widget/native_widget_aura.cc > File ui/views/widget/native_widget_aura.cc (right): > > https://codereview.chromium.org/1130033003/diff/140001/ui/views/widget/native_widget_aura.cc#newcode489 > ...
2 years, 6 months ago (2015-06-06 01:02:22 UTC) #50
joone
On 2015/06/06 01:02:22, joone wrote: > On 2015/06/06 00:47:35, oshima wrote: > > > https://codereview.chromium.org/1130033003/diff/140001/ui/views/widget/native_widget_aura.cc ...
2 years, 6 months ago (2015-06-06 01:50:58 UTC) #51
oshima
lgtm with a nit https://codereview.chromium.org/1130033003/diff/160001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/160001/ui/views/widget/native_widget_aura.cc#newcode499 ui/views/widget/native_widget_aura.cc:499: Minimize(); Can you add comment ...
2 years, 6 months ago (2015-06-08 17:20:13 UTC) #52
joone
On 2015/06/08 17:20:13, oshima wrote: > lgtm with a nit > > https://codereview.chromium.org/1130033003/diff/160001/ui/views/widget/native_widget_aura.cc > File ...
2 years, 6 months ago (2015-06-08 21:01:54 UTC) #55
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1130033003/180001
2 years, 6 months ago (2015-06-08 21:04:50 UTC) #56
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/69223)
2 years, 6 months ago (2015-06-08 21:16:58 UTC) #58
joone
On 2015/06/08 21:16:58, commit-bot: I haz the power wrote: > Try jobs failed on following ...
2 years, 6 months ago (2015-06-09 00:09:11 UTC) #59
oshima
sorry I should have said "i'm not the owner". you need approval from ui/views and ...
2 years, 6 months ago (2015-06-09 00:41:36 UTC) #60
not at google - send to devlin
Everybody seems happy, and I have zero to add either way - so lgtm.
2 years, 6 months ago (2015-06-10 17:27:03 UTC) #61
sadrul
Can you add a test?
2 years, 6 months ago (2015-06-10 17:34:47 UTC) #62
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1130033003/180001
2 years, 6 months ago (2015-06-10 18:15:16 UTC) #64
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/69860)
2 years, 6 months ago (2015-06-10 18:23:21 UTC) #66
joone
On 2015/06/10 17:34:47, sadrul wrote: > Can you add a test? There is already a ...
2 years, 6 months ago (2015-06-10 18:24:10 UTC) #67
joone
On 2015/06/10 17:27:03, kalman wrote: > Everybody seems happy, and I have zero to add ...
2 years, 6 months ago (2015-06-10 18:53:59 UTC) #68
limasdf
Joone, You still need lg from sadrul@. (tapted is owner of native_widget_mac* stuff.) Plus, I ...
2 years, 6 months ago (2015-06-10 19:03:44 UTC) #69
joone
On 2015/06/10 19:03:44, limasdf wrote: > Joone, You still need lg from sadrul@. (tapted is ...
2 years, 6 months ago (2015-06-12 21:17:56 UTC) #70
sadrul
https://codereview.chromium.org/1130033003/diff/200001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/200001/ui/views/widget/native_widget_aura_unittest.cc#newcode109 ui/views/widget/native_widget_aura_unittest.cc:109: window->ShowWithWindowState(ui::SHOW_STATE_MINIMIZED); Can you instead do: Widget::InitParams params(TYPE_WINDOW); params.show_state = ...
2 years, 6 months ago (2015-06-12 21:53:49 UTC) #71
joone
https://codereview.chromium.org/1130033003/diff/200001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/200001/ui/views/widget/native_widget_aura_unittest.cc#newcode109 ui/views/widget/native_widget_aura_unittest.cc:109: window->ShowWithWindowState(ui::SHOW_STATE_MINIMIZED); On 2015/06/12 21:53:49, sadrul wrote: > Can you ...
2 years, 6 months ago (2015-06-12 23:51:41 UTC) #72
varkha
https://codereview.chromium.org/1130033003/diff/220001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/220001/ui/views/widget/native_widget_aura_unittest.cc#newcode103 ui/views/widget/native_widget_aura_unittest.cc:103: TEST_F(NativeWidgetAuraTest, MinimizedStateTest) { Maybe s/MinimizedStateTest/CreateMinimized to be more specific?
2 years, 6 months ago (2015-06-19 16:14:57 UTC) #74
joone
https://codereview.chromium.org/1130033003/diff/220001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/220001/ui/views/widget/native_widget_aura_unittest.cc#newcode103 ui/views/widget/native_widget_aura_unittest.cc:103: TEST_F(NativeWidgetAuraTest, MinimizedStateTest) { On 2015/06/19 16:14:56, varkha wrote: > ...
2 years, 6 months ago (2015-06-19 20:18:39 UTC) #75
joone
On 2015/06/19 20:18:39, joone wrote: > https://codereview.chromium.org/1130033003/diff/220001/ui/views/widget/native_widget_aura_unittest.cc > File ui/views/widget/native_widget_aura_unittest.cc (right): > > https://codereview.chromium.org/1130033003/diff/220001/ui/views/widget/native_widget_aura_unittest.cc#newcode103 > ...
2 years, 5 months ago (2015-06-25 16:12:39 UTC) #76
sadrul
https://codereview.chromium.org/1130033003/diff/240001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/240001/ui/views/widget/native_widget_aura_unittest.cc#newcode113 ui/views/widget/native_widget_aura_unittest.cc:113: EXPECT_TRUE(widget->IsMinimized()); The change is in ShowWithWindowState(). Shouldn't this test ...
2 years, 5 months ago (2015-06-30 17:18:06 UTC) #77
joone
https://codereview.chromium.org/1130033003/diff/240001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/240001/ui/views/widget/native_widget_aura_unittest.cc#newcode113 ui/views/widget/native_widget_aura_unittest.cc:113: EXPECT_TRUE(widget->IsMinimized()); On 2015/06/30 17:18:05, sadrul wrote: > The change ...
2 years, 5 months ago (2015-06-30 23:51:40 UTC) #78
joone
https://codereview.chromium.org/1130033003/diff/260001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/260001/ui/views/widget/native_widget_aura_unittest.cc#newcode113 ui/views/widget/native_widget_aura_unittest.cc:113: widget->Minimize(); Now, this test case is almost like the ...
2 years, 5 months ago (2015-07-01 00:05:43 UTC) #79
sadrul
https://codereview.chromium.org/1130033003/diff/260001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/260001/ui/views/widget/native_widget_aura_unittest.cc#newcode113 ui/views/widget/native_widget_aura_unittest.cc:113: widget->Minimize(); On 2015/07/01 00:05:42, joone wrote: > Now, this ...
2 years, 5 months ago (2015-07-01 03:50:56 UTC) #80
joone
Please take a look at the updated CL. Thanks! https://codereview.chromium.org/1130033003/diff/260001/ui/views/widget/native_widget_aura_unittest.cc File ui/views/widget/native_widget_aura_unittest.cc (right): https://codereview.chromium.org/1130033003/diff/260001/ui/views/widget/native_widget_aura_unittest.cc#newcode113 ui/views/widget/native_widget_aura_unittest.cc:113: ...
2 years, 5 months ago (2015-07-01 22:50:15 UTC) #81
joone
sadrul@ ping?
2 years, 2 months ago (2015-09-28 07:23:58 UTC) #82
joone
2 years, 2 months ago (2015-09-30 02:26:38 UTC) #84
joone
sky@ can you review the test case?
2 years, 2 months ago (2015-09-30 02:28:55 UTC) #85
sadrul
Some of the test failures in the trybots look related. Mind taking a look at ...
2 years, 2 months ago (2015-09-30 04:03:51 UTC) #86
joone
sadrul@ All bots are now green. https://codereview.chromium.org/1130033003/diff/300001/ui/views/widget/widget.cc File ui/views/widget/widget.cc (right): https://codereview.chromium.org/1130033003/diff/300001/ui/views/widget/widget.cc#newcode628 ui/views/widget/widget.cc:628: if (IsFullscreen()) On ...
2 years, 2 months ago (2015-10-07 15:59:47 UTC) #87
sadrul
https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc#newcode487 ui/views/widget/native_widget_aura.cc:487: } In earlier patchsets, the property was being set ...
2 years, 2 months ago (2015-10-07 16:32:54 UTC) #88
varkha
https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc#newcode487 ui/views/widget/native_widget_aura.cc:487: } On 2015/10/07 16:32:54, sadrul wrote: > In earlier ...
2 years, 2 months ago (2015-10-07 18:16:57 UTC) #89
joone
https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc#newcode487 ui/views/widget/native_widget_aura.cc:487: } On 2015/10/07 16:32:54, sadrul wrote: > In earlier ...
2 years, 2 months ago (2015-10-16 07:29:49 UTC) #90
joone
On 2015/10/16 07:29:49, joone wrote: > https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc > File ui/views/widget/native_widget_aura.cc (right): > > https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc#newcode487 > ...
2 years, 1 month ago (2015-11-17 02:00:49 UTC) #91
oshima
On 2015/11/17 02:00:49, joone wrote: > On 2015/10/16 07:29:49, joone wrote: > > > https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc ...
2 years ago (2015-11-17 23:18:09 UTC) #92
oshima
https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc File ui/views/widget/native_widget_aura.cc (right): https://codereview.chromium.org/1130033003/diff/320001/ui/views/widget/native_widget_aura.cc#newcode487 ui/views/widget/native_widget_aura.cc:487: } On 2015/10/16 07:29:49, joone wrote: > On 2015/10/07 ...
2 years ago (2015-11-18 19:07:17 UTC) #93
joone
https://codereview.chromium.org/1130033003/diff/340001/ui/views/widget/widget.cc File ui/views/widget/widget.cc (right): https://codereview.chromium.org/1130033003/diff/340001/ui/views/widget/widget.cc#newcode635 ui/views/widget/widget.cc:635: if (IsFullscreen()) { On 2015/11/18 19:07:16, oshima wrote: > ...
2 years ago (2015-11-19 06:32:01 UTC) #94
joone
oshima@ could you review the updated CL?
2 years ago (2015-11-20 22:55:56 UTC) #95
oshima
On 2015/11/20 22:55:56, joone wrote: > oshima@ could you review the updated CL? I found ...
2 years ago (2015-11-20 23:16:39 UTC) #96
oshima
On 2015/11/20 23:16:39, oshima wrote: > On 2015/11/20 22:55:56, joone wrote: > > oshima@ could ...
2 years ago (2015-11-21 02:03:26 UTC) #97
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1130033003/360001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1130033003/360001
2 years ago (2015-11-21 04:43:32 UTC) #100
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/121038)
2 years ago (2015-11-21 04:54:00 UTC) #102
joone
sadrul@ devlin@ could you review this CL?
2 years ago (2015-11-21 05:05:11 UTC) #104
Devlin
extensions lgtm
2 years ago (2015-11-23 20:58:19 UTC) #105
sadrul
lgtm https://codereview.chromium.org/1130033003/diff/360001/ui/views/widget/widget.cc File ui/views/widget/widget.cc (right): https://codereview.chromium.org/1130033003/diff/360001/ui/views/widget/widget.cc#newcode641 ui/views/widget/widget.cc:641: (IsMinimized() ? ui::SHOW_STATE_MINIMIZED : saved_show_state_); Don't need ()
2 years ago (2015-11-25 20:44:32 UTC) #106
joone
Thanks! https://codereview.chromium.org/1130033003/diff/360001/ui/views/widget/widget.cc File ui/views/widget/widget.cc (right): https://codereview.chromium.org/1130033003/diff/360001/ui/views/widget/widget.cc#newcode641 ui/views/widget/widget.cc:641: (IsMinimized() ? ui::SHOW_STATE_MINIMIZED : saved_show_state_); On 2015/11/25 20:44:32, ...
2 years ago (2015-11-26 15:50:38 UTC) #109
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1130033003/380001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1130033003/380001
2 years ago (2015-11-26 15:56:16 UTC) #110
commit-bot: I haz the power
Committed patchset #20 (id:380001)
2 years ago (2015-11-26 16:06:12 UTC) #112
commit-bot: I haz the power
2 years ago (2015-11-26 16:07:57 UTC) #114
Message was sent while issue was closed.
Patchset 20 (id:??) landed as
https://crrev.com/d5d6762c5539bc7d6098588323a16a24826e1149
Cr-Commit-Position: refs/heads/master@{#361898}

Powered by Google App Engine
This is Rietveld 0eb02b776