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

Issue 2839993002: [Android] Adding Smart GO/NEXT feature in Chrome (Closed)

Created:
6 months, 4 weeks ago by AKVT
Modified:
5 months, 1 week ago
CC:
aelias_OOO_until_Jul13, agrieve+watch_chromium.org, blink-reviews, blink-reviews-api_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, dglazkov+blink, jam, kinuko+watch, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, Ted C
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Android] Adding Smart GO/NEXT feature in Chrome Smart Go/Next brings better user experience to the user during form submitting applications. For navigating between form elements, user can use NEXT/PREVIOUS button from IME without touching on individual fields. This will avoid unnecessary form submissions before filling or visiting all fields in the form. Additionally it will save user time and avoid redundant network requests before actually filling/attending entire fields in the form Design Document: https://docs.google.com/document/d/1h0diigZ8LUi7A3UKJ_zwNUbvNQoe-Nwr55_p6ivSPNg/edit?usp=sharing BUG=410785, 648986 Review-Url: https://codereview.chromium.org/2839993002 Cr-Commit-Position: refs/heads/master@{#479126} Committed: https://chromium.googlesource.com/chromium/src/+/8d80ad1410dcb23f55c323a97e57eed750a611dc

Patch Set 1 #

Total comments: 11

Patch Set 2 : Moved the triggering logic from WebView to WebFrame #

Patch Set 3 : Fixed WebViewTest build issues after restructure. #

Total comments: 6

Patch Set 4 : Rebased the patch along with review comment fixes. #

Total comments: 32

Patch Set 5 : Moved the Focus movement logic from IMC to FocusController and addressed other review comments. #

Total comments: 4

Patch Set 6 : Brought up blink enum to Java layer instead of boolean #

Total comments: 8

Patch Set 7 : Fixed FocusController and ImeAdapter related review comments. #

Total comments: 2

Patch Set 8 : Added few test related to Smart Go/next logic #

Total comments: 5

Patch Set 9 : testAdvanceFocusNextAndPrevious() is added for test coverage. #

Patch Set 10 : Reverted unnecessary touch in ImeTest. #

Total comments: 4

Patch Set 11 : Fixed ImeTest review comments. #

Total comments: 6

Patch Set 12 : Added inputType check in ImeTest #

Total comments: 30

Patch Set 13 : Fixed the ImeTest issues and added more coverage in WebViewTest #

Total comments: 9

Patch Set 14 : Fixed blink side review comments. #

Patch Set 15 : Rebased the patch for running android_fyi_perf bot. #

Patch Set 16 : Fixed the build break by replacing WebViewImpl object with WebViewBase #

Patch Set 17 : Rebased the patch and fixed ImeTest#testShowAndHideSoftInput #

Patch Set 18 : Rebased the patch for checking falky failures #

Patch Set 19 : Fixed testSelectActionBarClearedOnTappingOutsideInput failure #

Patch Set 20 : Rebased the patch and Fixed Webkit Layout test failure. #

Patch Set 21 : Rearranged the test HTML for fixing IME falky test. #

Patch Set 22 : Fixed BookmarksTest crash. #

Patch Set 23 : Fixed ImeTest#testSelectActionBarClearedOnTappingOutsideInput flaky failure #

Total comments: 5

Patch Set 24 : Preserved the DCHECK and fixed browser_tests failure #

Total comments: 3

Patch Set 25 : Rebased the patch #

Patch Set 26 : Rebased the patch from TOT #

Unified diffs Side-by-side diffs Delta from patch set Stats (+784 lines, -30 lines) Patch
M content/browser/android/ime_adapter_android.h View 1 2 3 4 5 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/android/ime_adapter_android.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +12 lines, -0 lines 0 comments Download
M content/common/frame_messages.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 1 chunk +6 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +24 lines, -8 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +15 lines, -14 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +7 lines, -3 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 2 chunks +64 lines, -0 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestInputMethodManagerWrapper.java View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +8 lines, -0 lines 0 comments Download
M content/renderer/render_frame_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +1 line, -0 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 3 chunks +8 lines, -0 lines 0 comments Download
M content/test/data/android/input/input_forms.html View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +9 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/editing/InputMethodController.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 2 chunks +10 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/FocusController.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/Source/core/page/FocusController.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 3 chunks +45 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebLocalFrameImpl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/WebLocalFrameImpl.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +16 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/web/tests/WebViewTest.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +483 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_disabled_and_readonly_elements.html View 1 chunk +11 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_key_event_listeners.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +31 lines, -0 lines 0 comments Download
A third_party/WebKit/Source/web/tests/data/advance_focus_in_form_with_tabindex_elements.html View 1 chunk +12 lines, -0 lines 0 comments Download
M third_party/WebKit/public/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 1 chunk +1 line, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebFocusType.h View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M third_party/WebKit/public/platform/WebTextInputType.h View 1 2 3 4 1 chunk +3 lines, -1 line 0 comments Download
M third_party/WebKit/public/web/WebLocalFrame.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 137 (38 generated)
AKVT
PTAL! In the meantime, could you help me to find the steps for running WebViewTest. ...
6 months, 4 weeks ago (2017-04-25 14:38:50 UTC) #8
nasko
On 2017/04/25 14:38:50, AKVT wrote: > PTAL! > > In the meantime, could you help ...
6 months, 3 weeks ago (2017-04-25 21:31:13 UTC) #9
nasko
Adding ekaramad@, who has done a lot of work recently on IME in the context ...
6 months, 3 weeks ago (2017-04-25 21:32:46 UTC) #11
EhsanK
I am not a good reviewer for core blink changes (InputMethodController). But I left some ...
6 months, 3 weeks ago (2017-04-25 22:43:08 UTC) #12
AKVT
On 2017/04/25 21:31:13, nasko wrote: > On 2017/04/25 14:38:50, AKVT wrote: > > PTAL! > ...
6 months, 3 weeks ago (2017-04-26 00:29:01 UTC) #13
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc File content/browser/android/ime_adapter_android.cc (right): https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc#newcode314 content/browser/android/ime_adapter_android.cc:314: rfh->Send(new FrameMsg_AdvanceFocusInForm(rfh->GetRoutingID(), forward)); On 2017/04/25 22:43:08, EhsanK wrote: ...
6 months, 3 weeks ago (2017-04-26 10:33:04 UTC) #14
EhsanK
Thanks for addressing the issues! LGTM with suggestions. https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc File content/browser/android/ime_adapter_android.cc (right): https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc#newcode314 content/browser/android/ime_adapter_android.cc:314: rfh->Send(new ...
6 months, 3 weeks ago (2017-04-26 21:31:59 UTC) #15
kochi
Did you rebase after the Blink big renaming? (which happened around early this month)
6 months, 3 weeks ago (2017-04-27 02:03:17 UTC) #16
AKVT
On 2017/04/27 02:03:17, kochi wrote: > Did you rebase after the Blink big renaming? > ...
6 months, 3 weeks ago (2017-04-27 03:44:31 UTC) #17
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc File content/browser/android/ime_adapter_android.cc (right): https://codereview.chromium.org/2839993002/diff/1/content/browser/android/ime_adapter_android.cc#newcode314 content/browser/android/ime_adapter_android.cc:314: rfh->Send(new FrameMsg_AdvanceFocusInForm(rfh->GetRoutingID(), forward)); On 2017/04/26 21:31:59, EhsanK wrote: ...
6 months, 3 weeks ago (2017-04-27 15:07:02 UTC) #18
dcheng
Also, please address kochi's comment about naming in Blink. Blink mostly follows Chrome's naming conventions ...
6 months, 3 weeks ago (2017-04-27 15:33:03 UTC) #19
Changwan Ryu
Please add some tests to ImeTest.java . https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java File content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java (right): https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java#newcode625 content/public/android/java/src/org/chromium/content/browser/input/ImeAdapter.java:625: restartInput(); I ...
6 months, 3 weeks ago (2017-04-27 16:34:54 UTC) #20
Changwan Ryu
also adding yosin@ for InputMethodController. yosin@ and kochi@, does it make sense for IMC to ...
6 months, 3 weeks ago (2017-04-27 16:44:18 UTC) #22
yosin_UTC9
Could you provide design doc? I'm not sure what "Smart Go NEXT feature." Thanks in ...
6 months, 3 weeks ago (2017-04-28 01:07:28 UTC) #23
kochi
On 2017/04/28 01:07:28, yosin_OOO_til_May_7 wrote: > Could you provide design doc? I'm not sure what ...
6 months, 3 weeks ago (2017-04-28 01:14:33 UTC) #24
yosin_UTC9
On 2017/04/28 at 01:14:33, kochi wrote: > On 2017/04/28 01:07:28, yosin_OOO_til_May_7 wrote: > > Could ...
6 months, 3 weeks ago (2017-04-28 03:30:16 UTC) #25
yosin_UTC9
https://codereview.chromium.org/2839993002/diff/60001/content/renderer/render_frame_impl.cc File content/renderer/render_frame_impl.cc (right): https://codereview.chromium.org/2839993002/diff/60001/content/renderer/render_frame_impl.cc#newcode2244 content/renderer/render_frame_impl.cc:2244: void RenderFrameImpl::OnAdvanceFocusInForm(bool forward) { Please avoid to use |bool| ...
6 months, 3 weeks ago (2017-04-28 03:32:11 UTC) #26
AKVT
PTAL! I am in process of writing few tests in ImeTest.java and will upload in ...
6 months, 2 weeks ago (2017-05-03 14:35:12 UTC) #27
nasko
The C++ parts of content/ look mostly good. One recommendation on the IPC and I'll ...
6 months, 2 weeks ago (2017-05-03 23:53:25 UTC) #28
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/80001/content/common/frame_messages.h File content/common/frame_messages.h (right): https://codereview.chromium.org/2839993002/diff/80001/content/common/frame_messages.h#newcode967 content/common/frame_messages.h:967: // Tells the renderer to advance the focus ...
6 months, 2 weeks ago (2017-05-04 17:08:04 UTC) #29
nasko
Thanks! content/ (minus java parts) LGTM.
6 months, 2 weeks ago (2017-05-04 17:22:28 UTC) #30
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java File content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java (right): https://codereview.chromium.org/2839993002/diff/60001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java#newcode125 content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java:125: boolean enterKeyAction = false; On 2017/05/03 14:35:12, AKVT wrote: ...
6 months, 2 weeks ago (2017-05-04 20:38:16 UTC) #31
yosin_UTC9
core/editing/InputMethodController.cpp lgtm
6 months, 2 weeks ago (2017-05-08 03:40:54 UTC) #32
kochi
Reviewed FocusController.*. Could you also add a link to your design doc in the CL ...
6 months, 2 weeks ago (2017-05-09 02:47:43 UTC) #33
kochi
On 2017/04/25 21:31:13, nasko wrote: > On 2017/04/25 14:38:50, AKVT wrote: > > PTAL! > ...
6 months, 2 weeks ago (2017-05-09 02:51:36 UTC) #34
AKVT
On 2017/05/09 02:51:36, kochi wrote: > On 2017/04/25 21:31:13, nasko wrote: > > On 2017/04/25 ...
6 months, 2 weeks ago (2017-05-09 08:14:22 UTC) #35
AKVT
PTAL! @changwan - May plan for testing in ImeTest is as below; 1) Modify the ...
6 months, 2 weeks ago (2017-05-09 11:05:53 UTC) #37
Changwan Ryu
Regarding testing approach, Please do NOT try to inject fake focus IDs. You can focus ...
6 months, 2 weeks ago (2017-05-09 14:43:58 UTC) #38
AKVT
On 2017/05/09 14:43:58, Changwan Ryu wrote: > Regarding testing approach, > > Please do NOT ...
6 months, 2 weeks ago (2017-05-09 14:52:49 UTC) #39
Changwan Ryu
On 2017/05/09 14:52:49, AKVT wrote: > On 2017/05/09 14:43:58, Changwan Ryu wrote: > > Regarding ...
6 months, 2 weeks ago (2017-05-09 15:02:33 UTC) #40
Changwan Ryu
BTW, could you describe your CL in more details in the commit subject / description? ...
6 months, 2 weeks ago (2017-05-09 17:07:46 UTC) #41
AKVT
PTAL! @changwan - Regarding test failure, could you give some initial inputs ? https://codereview.chromium.org/2839993002/diff/120001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java File ...
6 months, 2 weeks ago (2017-05-09 17:22:18 UTC) #44
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode43 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:43: EditorInfo.IME_FLAG_NO_FULLSCREEN | EditorInfo.IME_FLAG_NO_EXTRACT_UI; Instead of make another dependency on ...
6 months, 1 week ago (2017-05-09 18:12:16 UTC) #47
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/140001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode43 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:43: EditorInfo.IME_FLAG_NO_FULLSCREEN | EditorInfo.IME_FLAG_NO_EXTRACT_UI; On 2017/05/09 18:12:16, Changwan Ryu ...
6 months, 1 week ago (2017-05-10 09:31:38 UTC) #48
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode423 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:423: // Form1 forward and backward focus Do we really ...
6 months, 1 week ago (2017-05-10 16:20:59 UTC) #49
kochi
FocusController.* lgtm
6 months, 1 week ago (2017-05-11 02:27:21 UTC) #50
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/180001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode423 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:423: // Form1 forward and backward focus On 2017/05/10 ...
6 months, 1 week ago (2017-05-11 08:47:39 UTC) #51
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode437 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:437: Assert.assertTrue((EditorInfo.IME_ACTION_NEXT & getImeActions()) != 0); This is somewhat concerning. ...
6 months, 1 week ago (2017-05-11 09:10:52 UTC) #52
AKVT
I doubt we need to get the data from computeEditorInfo() and make available for querying. ...
6 months, 1 week ago (2017-05-11 09:48:49 UTC) #53
AKVT
PTAL! Added inputType check in tests.
6 months, 1 week ago (2017-05-11 10:09:35 UTC) #54
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java#newcode437 content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java:437: Assert.assertTrue((EditorInfo.IME_ACTION_NEXT & getImeActions()) != 0); On 2017/05/11 09:48:49, AKVT ...
6 months, 1 week ago (2017-05-11 12:04:38 UTC) #55
AKVT
@changwan - Please address my query and continue review on PS12 https://codereview.chromium.org/2839993002/diff/200001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeTest.java (right): ...
6 months, 1 week ago (2017-05-11 12:16:44 UTC) #56
Changwan Ryu
On 2017/05/11 12:16:44, AKVT wrote: > @changwan - Please address my query and continue review ...
6 months, 1 week ago (2017-05-11 15:32:21 UTC) #57
Changwan Ryu
6 months, 1 week ago (2017-05-11 15:32:38 UTC) #58
AKVT
On 2017/05/11 15:32:21, Changwan Ryu wrote: > On 2017/05/11 12:16:44, AKVT wrote: > > @changwan ...
6 months, 1 week ago (2017-05-11 15:49:41 UTC) #59
Changwan Ryu
On 2017/05/11 15:49:41, AKVT wrote: > On 2017/05/11 15:32:21, Changwan Ryu wrote: > > On ...
6 months, 1 week ago (2017-05-11 15:55:26 UTC) #60
AKVT
On 2017/05/11 15:55:26, Changwan Ryu wrote: > On 2017/05/11 15:49:41, AKVT wrote: > > On ...
6 months, 1 week ago (2017-05-11 16:13:05 UTC) #61
Changwan Ryu
To answer the previous question, advancing the focus takes a round trip to the Blink ...
6 months, 1 week ago (2017-05-11 22:57:13 UTC) #62
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/220001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java File content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java (right): https://codereview.chromium.org/2839993002/diff/220001/content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java#newcode65 content/public/android/java/src/org/chromium/content/browser/input/ImeUtils.java:65: imeAction = EditorInfo.IME_ACTION_SEARCH; On 2017/05/11 22:57:12, Changwan Ryu ...
6 months, 1 week ago (2017-05-12 13:26:26 UTC) #63
Changwan Ryu
lgtm https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java (right): https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java#newcode573 content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java:573: return mFactory.initializeAndGet(view, imeAdapter, inputType, inputFlags, inputMode, On 2017/05/12 ...
6 months, 1 week ago (2017-05-12 22:22:02 UTC) #64
AKVT
@nasko and @dcheng - PTAL content/common/frame_messages.h changes as it needs SECURITY_OWNERS stamping. @tkent - PTAL ...
6 months, 1 week ago (2017-05-13 04:00:46 UTC) #65
dcheng
I think nasko already approved, but ipc LGTM as well. https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java File content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java (right): https://codereview.chromium.org/2839993002/diff/240001/content/public/android/javatests/src/org/chromium/content/browser/input/ImeActivityTestRule.java#newcode573 ...
6 months, 1 week ago (2017-05-14 02:53:21 UTC) #66
tkent
The previous CL was reverted due to a power regression [1]. Does this CL address ...
6 months, 1 week ago (2017-05-14 23:24:59 UTC) #67
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/240001/third_party/WebKit/Source/core/page/FocusController.cpp File third_party/WebKit/Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/2839993002/diff/240001/third_party/WebKit/Source/core/page/FocusController.cpp#newcode1102 third_party/WebKit/Source/core/page/FocusController.cpp:1102: // TODO(ajith.v) Extend it for Select element, Radio ...
6 months, 1 week ago (2017-05-15 06:40:42 UTC) #68
AKVT
On 2017/05/14 23:24:59, tkent wrote: > The previous CL was reverted due to a power ...
6 months, 1 week ago (2017-05-15 06:43:03 UTC) #69
AKVT
@tkent & @kochi - I initiated try jobs for measuring power.android_acceptance in android port. Please ...
6 months, 1 week ago (2017-05-15 13:06:14 UTC) #70
Changwan Ryu
On 2017/05/15 13:06:14, AKVT wrote: > @tkent & @kochi - I initiated try jobs for ...
6 months, 1 week ago (2017-05-16 04:56:06 UTC) #75
AKVT
On 2017/05/16 04:56:06, Changwan Ryu wrote: > On 2017/05/15 13:06:14, AKVT wrote: > > @tkent ...
6 months, 1 week ago (2017-05-16 06:52:38 UTC) #76
AKVT
@changwan, @kochi and @tkent - Suggested test run is completed by bot now. Most of ...
6 months ago (2017-05-17 06:48:03 UTC) #77
Changwan Ryu
On 2017/05/17 06:48:03, AKVT wrote: > @changwan, @kochi and @tkent - Suggested test run is ...
6 months ago (2017-05-17 14:29:50 UTC) #78
AKVT
On 2017/05/17 14:29:50, Changwan Ryu wrote: > On 2017/05/17 06:48:03, AKVT wrote: > > @changwan, ...
6 months ago (2017-05-17 14:41:04 UTC) #79
Changwan Ryu
On 2017/05/17 14:41:04, AKVT wrote: > On 2017/05/17 14:29:50, Changwan Ryu wrote: > > On ...
6 months ago (2017-05-18 01:11:05 UTC) #80
tkent
> tkent@, would it be ok to let this land and have it reverted in ...
6 months ago (2017-05-18 23:23:32 UTC) #81
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/280001
6 months ago (2017-05-19 03:47:04 UTC) #84
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/430211) linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED, ...
6 months ago (2017-05-19 04:28:04 UTC) #86
AKVT
On 2017/05/19 04:28:04, commit-bot: I haz the power wrote: > Try jobs failed on following ...
6 months ago (2017-05-19 10:45:54 UTC) #87
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/300001
6 months ago (2017-05-19 10:46:41 UTC) #90
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_chromeos_rel_ng/builds/430421)
6 months ago (2017-05-19 11:59:07 UTC) #92
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/300001
6 months ago (2017-05-19 18:31:57 UTC) #94
commit-bot: I haz the power
Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_swarming_rel/builds/182517)
6 months ago (2017-05-19 19:42:49 UTC) #96
AKVT
@changwan - After rebase I saw testShowAndHideSoftInput is failing, so I preserved resetUpdateSelectionList() method. PTAL!
5 months, 3 weeks ago (2017-05-30 14:54:46 UTC) #97
Changwan Ryu
On 2017/05/30 14:54:46, AKVT wrote: > @changwan - After rebase I saw testShowAndHideSoftInput is failing, ...
5 months, 3 weeks ago (2017-05-30 20:04:08 UTC) #98
AKVT
On 2017/05/30 20:04:08, Changwan Ryu wrote: > On 2017/05/30 14:54:46, AKVT wrote: > > @changwan ...
5 months, 3 weeks ago (2017-05-31 12:34:49 UTC) #99
AKVT
Unfortunately, none of the failures are not able to reproduce locally. ImeTest, AutoFillPopupTest etc. all ...
5 months, 2 weeks ago (2017-06-02 10:35:30 UTC) #100
AKVT
@kochi & @changwan - PTAL. I have removed a DCHECK from Element.cpp for solving launch ...
5 months, 2 weeks ago (2017-06-06 18:14:02 UTC) #101
kochi
@changwan - probably you are better at looking at this recent changes. could you take ...
5 months, 1 week ago (2017-06-09 03:53:25 UTC) #102
AKVT
PTAL - PS23 I have fixed the flaky failures in ImeTest.
5 months, 1 week ago (2017-06-09 05:11:21 UTC) #103
Changwan Ryu
I have hard time diff'ing patch 22 and patch 23 for the change you made. ...
5 months, 1 week ago (2017-06-09 15:40:04 UTC) #104
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/page/FocusController.cpp File third_party/WebKit/Source/core/page/FocusController.cpp (right): https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/page/FocusController.cpp#newcode1072 third_party/WebKit/Source/core/page/FocusController.cpp:1072: WebFocusType focus_type) { could you try adding element->GetDocument().UpdateStyleAndLayoutIgnorePendingStylesheets(); at ...
5 months, 1 week ago (2017-06-09 19:34:30 UTC) #105
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
5 months, 1 week ago (2017-06-10 03:34:29 UTC) #106
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
5 months, 1 week ago (2017-06-10 03:34:32 UTC) #107
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
5 months, 1 week ago (2017-06-10 03:34:34 UTC) #108
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
5 months, 1 week ago (2017-06-10 03:34:38 UTC) #109
AKVT
On 2017/06/09 15:40:04, Changwan Ryu wrote: > I have hard time diff'ing patch 22 and ...
5 months, 1 week ago (2017-06-10 03:34:39 UTC) #110
AKVT
@changwan - i will try to call UpdateStyleAndLayoutIgnorePendingStylesheets() and share result in next patch. Thank ...
5 months, 1 week ago (2017-06-10 03:37:56 UTC) #111
AKVT
PTAL! https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/dom/Element.cpp File third_party/WebKit/Source/core/dom/Element.cpp (left): https://codereview.chromium.org/2839993002/diff/440001/third_party/WebKit/Source/core/dom/Element.cpp#oldcode2897 third_party/WebKit/Source/core/dom/Element.cpp:2897: !GetDocument().NeedsLayoutTreeUpdateForNode(*this)); On 2017/06/09 15:40:04, Changwan Ryu wrote: > ...
5 months, 1 week ago (2017-06-12 12:21:24 UTC) #112
Changwan Ryu
https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html File content/test/data/android/input/input_forms.html (right): https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html#newcode25 content/test/data/android/input/input_forms.html:25: <input id="input_radio" type="radio" style="width:50px;height:50px" /><br/> could you explain why ...
5 months, 1 week ago (2017-06-12 20:47:24 UTC) #113
AKVT
@changwan - PTAL https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html File content/test/data/android/input/input_forms.html (right): https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html#newcode25 content/test/data/android/input/input_forms.html:25: <input id="input_radio" type="radio" style="width:50px;height:50px" /><br/> On ...
5 months, 1 week ago (2017-06-13 06:41:50 UTC) #114
Changwan Ryu
lgtm https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html File content/test/data/android/input/input_forms.html (right): https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html#newcode25 content/test/data/android/input/input_forms.html:25: <input id="input_radio" type="radio" style="width:50px;height:50px" /><br/> On 2017/06/13 06:41:49, ...
5 months, 1 week ago (2017-06-13 15:43:27 UTC) #115
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/460001
5 months, 1 week ago (2017-06-13 15:48:53 UTC) #118
commit-bot: I haz the power
Try jobs failed on following builders: ios-simulator on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/builds/236410)
5 months, 1 week ago (2017-06-13 15:52:28 UTC) #120
AKVT
On 2017/06/13 15:43:27, Changwan Ryu wrote: > lgtm > > https://codereview.chromium.org/2839993002/diff/460001/content/test/data/android/input/input_forms.html > File content/test/data/android/input/input_forms.html (right): ...
5 months, 1 week ago (2017-06-13 15:59:40 UTC) #121
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/480001
5 months, 1 week ago (2017-06-13 16:00:33 UTC) #124
commit-bot: I haz the power
Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_compile_dbg_ng/builds/440467) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, ...
5 months, 1 week ago (2017-06-13 16:03:19 UTC) #126
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/500001
5 months, 1 week ago (2017-06-13 16:10:08 UTC) #129
commit-bot: I haz the power
Try jobs failed on following builders: android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm64_dbg_recipe/builds/287907) android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED, ...
5 months, 1 week ago (2017-06-13 17:00:10 UTC) #131
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2839993002/500001
5 months, 1 week ago (2017-06-13 19:36:01 UTC) #133
commit-bot: I haz the power
Committed patchset #26 (id:500001) as https://chromium.googlesource.com/chromium/src/+/8d80ad1410dcb23f55c323a97e57eed750a611dc
5 months, 1 week ago (2017-06-13 20:21:32 UTC) #136
kochi
5 months, 1 week ago (2017-06-15 01:47:17 UTC) #137
Message was sent while issue was closed.
A revert of this CL (patchset #26 id:500001) has been created in
https://codereview.chromium.org/2938123002/ by kochi@chromium.org.

The reason for reverting is: This caused several clusterfuzz bugs
(733197, 733218, 733282) - let me revert for
investigation on those..

Powered by Google App Engine
This is Rietveld efc10ee0f