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

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

Created:
1 year, 1 month ago by AKVT
Modified:
1 year 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. ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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! > ...
1 year, 1 month 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: ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-04-26 21:31:59 UTC) #15
kochi
Did you rebase after the Blink big renaming? (which happened around early this month)
1 year, 1 month 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? > ...
1 year, 1 month 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: ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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| ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-04 17:08:04 UTC) #29
nasko
Thanks! content/ (minus java parts) LGTM.
1 year, 1 month 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: ...
1 year, 1 month ago (2017-05-04 20:38:16 UTC) #31
yosin_UTC9
core/editing/InputMethodController.cpp lgtm
1 year, 1 month 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 ...
1 year, 1 month 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! > ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-09 08:14:22 UTC) #35
AKVT
PTAL! @changwan - May plan for testing in ImeTest is as below; 1) Modify the ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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? ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-10 16:20:59 UTC) #49
kochi
FocusController.* lgtm
1 year, 1 month 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 ...
1 year, 1 month 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. ...
1 year, 1 month 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. ...
1 year, 1 month ago (2017-05-11 09:48:49 UTC) #53
AKVT
PTAL! Added inputType check in tests.
1 year, 1 month 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 ...
1 year, 1 month 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): ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-11 15:32:21 UTC) #57
Changwan Ryu
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-16 06:52:38 UTC) #76
AKVT
@changwan, @kochi and @tkent - Suggested test run is completed by bot now. Most of ...
1 year, 1 month 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 ...
1 year, 1 month 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, ...
1 year, 1 month 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 ...
1 year, 1 month ago (2017-05-18 01:11:05 UTC) #80
tkent
> tkent@, would it be ok to let this land and have it reverted in ...
1 year, 1 month 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
1 year 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, ...
1 year 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 ...
1 year 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
1 year 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)
1 year 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
1 year 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)
1 year ago (2017-05-19 19:42:49 UTC) #96
AKVT
@changwan - After rebase I saw testShowAndHideSoftInput is failing, so I preserved resetUpdateSelectionList() method. PTAL!
1 year 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, ...
1 year 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 ...
1 year 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 ...
1 year ago (2017-06-02 10:35:30 UTC) #100
AKVT
@kochi & @changwan - PTAL. I have removed a DCHECK from Element.cpp for solving launch ...
1 year ago (2017-06-06 18:14:02 UTC) #101
kochi
@changwan - probably you are better at looking at this recent changes. could you take ...
1 year ago (2017-06-09 03:53:25 UTC) #102
AKVT
PTAL - PS23 I have fixed the flaky failures in ImeTest.
1 year 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. ...
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year 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 ...
1 year ago (2017-06-10 03:34:39 UTC) #110
AKVT
@changwan - i will try to call UpdateStyleAndLayoutIgnorePendingStylesheets() and share result in next patch. Thank ...
1 year 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: > ...
1 year 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 ...
1 year 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 ...
1 year 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, ...
1 year 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
1 year 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)
1 year 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): ...
1 year 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
1 year 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, ...
1 year 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
1 year 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, ...
1 year 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
1 year 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
1 year ago (2017-06-13 20:21:32 UTC) #136
kochi
1 year 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 408576698