Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(7)

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

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