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

Issue 2908613002: Move common functionality of IOSChromeIOThread to ios/components. (Closed)

Created:
3 years, 7 months ago by michaeldo
Modified:
3 years, 6 months ago
CC:
chromium-reviews, ios-reviews+chrome_chromium.org, ios-reviews_chromium.org, droger+watchlist_chromium.org, blundell+watchlist_chromium.org, pkl (ping after 24h if needed), sdefresne+watchlist_chromium.org, noyau+watch_chromium.org, marq+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Move common functionality of IOSChromeIOThread to ios/components. Most of this functionality is not reliant on ios/chrome. Moving it to ios/components allows it to be used by ios/web_view. The existing unittest is left in place because it relies on the setup that happens in the IOSChromeUnitTestSuite class. BUG=725524 Review-Url: https://codereview.chromium.org/2908613002 Cr-Commit-Position: refs/heads/master@{#475627} Committed: https://chromium.googlesource.com/chromium/src/+/5e8fc527dd73f46682283810a01445672e475b80

Patch Set 1 #

Patch Set 2 : Remove bug todo. #

Total comments: 12

Patch Set 3 : Respond to comments. #

Total comments: 6

Patch Set 4 : Respond to comments. #

Total comments: 18

Patch Set 5 : Respond to comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+302 lines, -881 lines) Patch
M ios/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/BUILD.gn View 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/DEPS View 1 chunk +1 line, -0 lines 0 comments Download
M ios/chrome/browser/ios_chrome_io_thread.h View 1 2 3 4 1 chunk +6 lines, -206 lines 0 comments Download
M ios/chrome/browser/ios_chrome_io_thread.mm View 1 2 3 4 1 chunk +7 lines, -554 lines 0 comments Download
A ios/components/BUILD.gn View 1 chunk +33 lines, -0 lines 0 comments Download
A ios/components/DEPS View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download
A ios/components/README.md View 1 2 3 4 1 chunk +12 lines, -0 lines 0 comments Download
A ios/components/io_thread/BUILD.gn View 1 2 1 chunk +48 lines, -0 lines 0 comments Download
A ios/components/io_thread/DEPS View 1 chunk +11 lines, -0 lines 0 comments Download
A + ios/components/io_thread/ios_io_thread.h View 1 2 3 4 10 chunks +34 lines, -19 lines 0 comments Download
A + ios/components/io_thread/ios_io_thread.mm View 1 2 3 4 23 chunks +55 lines, -102 lines 0 comments Download
A ios/components/io_thread/ios_io_thread_unittest.mm View 1 2 3 4 1 chunk +81 lines, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 26 (10 generated)
michaeldo
3 years, 7 months ago (2017-05-25 21:21:59 UTC) #3
Eugene But (OOO till 7-30)
https://codereview.chromium.org/2908613002/diff/20001/ios/chrome/browser/ios_chrome_io_thread.mm File ios/chrome/browser/ios_chrome_io_thread.mm (right): https://codereview.chromium.org/2908613002/diff/20001/ios/chrome/browser/ios_chrome_io_thread.mm#newcode11 ios/chrome/browser/ios_chrome_io_thread.mm:11: //#include "ios/chrome/browser/chrome_switches.h" Do you need these ? https://codereview.chromium.org/2908613002/diff/20001/ios/components/README File ...
3 years, 7 months ago (2017-05-25 22:05:22 UTC) #4
michaeldo
https://codereview.chromium.org/2908613002/diff/20001/ios/chrome/browser/ios_chrome_io_thread.mm File ios/chrome/browser/ios_chrome_io_thread.mm (right): https://codereview.chromium.org/2908613002/diff/20001/ios/chrome/browser/ios_chrome_io_thread.mm#newcode11 ios/chrome/browser/ios_chrome_io_thread.mm:11: //#include "ios/chrome/browser/chrome_switches.h" On 2017/05/25 22:05:21, Eugene But wrote: > ...
3 years, 7 months ago (2017-05-26 07:24:44 UTC) #5
Eugene But (OOO till 7-30)
lgtm https://codereview.chromium.org/2908613002/diff/40001/ios/components/DEPS File ios/components/DEPS (right): https://codereview.chromium.org/2908613002/diff/40001/ios/components/DEPS#newcode4 ios/components/DEPS:4: "-ios/chrome", "-ios/clean", ? https://codereview.chromium.org/2908613002/diff/40001/ios/components/io_thread/ios_io_thread.h File ios/components/io_thread/ios_io_thread.h (right): https://codereview.chromium.org/2908613002/diff/40001/ios/components/io_thread/ios_io_thread.h#newcode161 ...
3 years, 7 months ago (2017-05-26 14:18:10 UTC) #6
michaeldo
https://codereview.chromium.org/2908613002/diff/40001/ios/components/DEPS File ios/components/DEPS (right): https://codereview.chromium.org/2908613002/diff/40001/ios/components/DEPS#newcode4 ios/components/DEPS:4: "-ios/chrome", On 2017/05/26 14:18:09, Eugene But wrote: > "-ios/clean", ...
3 years, 7 months ago (2017-05-26 17:19:06 UTC) #7
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/2908613002/60001
3 years, 7 months ago (2017-05-26 17:21:26 UTC) #10
commit-bot: I haz the power
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presubmit/builds/448868)
3 years, 7 months ago (2017-05-26 17:29:35 UTC) #12
michaeldo
On 2017/05/26 17:29:35, commit-bot: I haz the power wrote: > Try jobs failed on following ...
3 years, 7 months ago (2017-05-26 17:37:48 UTC) #13
mmenke
On 2017/05/26 17:37:48, michaeldo wrote: > On 2017/05/26 17:29:35, commit-bot: I haz the power wrote: ...
3 years, 7 months ago (2017-05-26 17:40:32 UTC) #14
blundell
If there are now tests in ios_chrome_io_thread_unittest.cc that are really testing IOSIOThread, please ensure that ...
3 years, 6 months ago (2017-05-29 07:21:16 UTC) #16
sdefresne
lgtm with comments https://codereview.chromium.org/2908613002/diff/60001/ios/chrome/browser/ios_chrome_io_thread.mm File ios/chrome/browser/ios_chrome_io_thread.mm (right): https://codereview.chromium.org/2908613002/diff/60001/ios/chrome/browser/ios_chrome_io_thread.mm#newcode18 ios/chrome/browser/ios_chrome_io_thread.mm:18: IOSChromeNetworkDelegate::InitializePrefsOnUIThread(nullptr, local_state); The method IOSChromeNetworkDelegate::InitializePrefsOnUIThread does ...
3 years, 6 months ago (2017-05-29 09:30:41 UTC) #17
sdefresne
https://codereview.chromium.org/2908613002/diff/60001/ios/chrome/browser/ios_chrome_io_thread.mm File ios/chrome/browser/ios_chrome_io_thread.mm (right): https://codereview.chromium.org/2908613002/diff/60001/ios/chrome/browser/ios_chrome_io_thread.mm#newcode18 ios/chrome/browser/ios_chrome_io_thread.mm:18: IOSChromeNetworkDelegate::InitializePrefsOnUIThread(nullptr, local_state); On 2017/05/29 09:30:40, sdefresne wrote: > The ...
3 years, 6 months ago (2017-05-29 09:41:01 UTC) #18
michaeldo
On 2017/05/29 07:21:16, blundell wrote: > If there are now tests in ios_chrome_io_thread_unittest.cc that are ...
3 years, 6 months ago (2017-05-30 17:16:15 UTC) #19
michaeldo
https://codereview.chromium.org/2908613002/diff/60001/ios/components/DEPS File ios/components/DEPS (right): https://codereview.chromium.org/2908613002/diff/60001/ios/components/DEPS#newcode4 ios/components/DEPS:4: "-ios/chrome", On 2017/05/29 09:30:40, sdefresne wrote: > I would ...
3 years, 6 months ago (2017-05-30 17:16:24 UTC) #20
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/2908613002/80001
3 years, 6 months ago (2017-05-30 17:17:00 UTC) #23
commit-bot: I haz the power
3 years, 6 months ago (2017-05-30 19:18:38 UTC) #26
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/5e8fc527dd73f46682283810a014...

Powered by Google App Engine
This is Rietveld 408576698