Migrate WebThreadImpl et al. to match new BrowserThreadImpl et al. |
||||
Issue description//content just went through a major cleanup in this space Refactor : https://chromium-review.googlesource.com/c/chromium/src/+/980793 Remove BrowserThread lock: https://chromium-review.googlesource.com/c/chromium/src/+/973556 The same can be applied to WebThreads but this is too big of a change for me to copy/paste in without the ability to compile. @sdefresne to perform this migration, thanks! When this is done, I'll proceed with cleanup of base::Thread::SetMessageLoop which will no longer be used :)
,
Nov 12
Hi Sylvain, //ios lagging behind here is now blocking https://chromium-review.googlesource.com/c/chromium/src/+/1320329 and follow-up important refactors in //base. Not having https://chromium-review.googlesource.com/c/chromium/src/+/973556 on the //ios side means it suffers from issue 821034 (and thus issue 890978 as well). What's the ios team's strategy here? I find it unfortunate to have this copy/pasted code from //content for all the threading code... could it not have been made into a component like other shared code? Apart from the naming of UI/IO thread enums, there isn't anything particularly //content specific about this code... It being in an ios specific section of the codebase means we can't reasonably keep it up to date as the majority of us can't even compile ios locally so clearly won't engage in a refactoring...
,
Nov 12
,
Nov 12
Doing this blocks getting the benefits of https://chromium-review.googlesource.com/973556 on iOS to fix issue 821034 and hence issue 890978 there.
,
Nov 12
Spoke with Rohit offline, he will be picking this up. Thanks Rohit!
,
Nov 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8532e551ec71212df79dc674a8cee0dc1f306d73 commit 8532e551ec71212df79dc674a8cee0dc1f306d73 Author: Rohit Rao <rohitrao@chromium.org> Date: Thu Nov 15 13:12:35 2018 [ios] Clarifies that WebThread::SetDelegate can only be called for the IO thread. This is the //web counterpart to https://codereview.chromium.org/2558943002. BUG=826465 Change-Id: I83e09e189ad0cbbdccaf9fa0de104efa7a6386a3 Reviewed-on: https://chromium-review.googlesource.com/c/1333898 Commit-Queue: Rohit Rao <rohitrao@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#608342} [modify] https://crrev.com/8532e551ec71212df79dc674a8cee0dc1f306d73/ios/components/io_thread/ios_io_thread.mm [modify] https://crrev.com/8532e551ec71212df79dc674a8cee0dc1f306d73/ios/web/public/web_thread.h [modify] https://crrev.com/8532e551ec71212df79dc674a8cee0dc1f306d73/ios/web/public/web_thread_delegate.h [modify] https://crrev.com/8532e551ec71212df79dc674a8cee0dc1f306d73/ios/web/web_thread_impl.cc
,
Nov 16
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ecad200a38049d02792b605cd9ee1f31220a9191 commit ecad200a38049d02792b605cd9ee1f31220a9191 Author: Rohit Rao <rohitrao@chromium.org> Date: Fri Nov 16 19:42:05 2018 [ios] Creates WebSubThread to mirror BrowserProcessSubThread. Moves IO thread-specific initialization into WebSubThread. BUG=826465 Change-Id: I5527dafdf75c29fbc8478f7500c20a96410ad008 Reviewed-on: https://chromium-review.googlesource.com/c/1335961 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Commit-Queue: Rohit Rao <rohitrao@chromium.org> Cr-Commit-Position: refs/heads/master@{#608898} [modify] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/BUILD.gn [modify] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/app/web_main_loop.h [modify] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/app/web_main_loop.mm [add] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/web_sub_thread.cc [add] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/web_sub_thread.h [modify] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/web_thread_impl.cc [modify] https://crrev.com/ecad200a38049d02792b605cd9ee1f31220a9191/ios/web/web_thread_impl.h
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d3f6bf113b71a9d0d2a259152581a5d65f76a22 commit 3d3f6bf113b71a9d0d2a259152581a5d65f76a22 Author: Rohit Rao <rohitrao@chromium.org> Date: Thu Dec 13 15:07:50 2018 [ios] Store TaskRunners instead of threads in WebThreadImpl. Track thread states explicitly instead of simply checking whether a TaskRunner pointer exists or not. BUG=826465 Change-Id: Id0f84e7eedc6b9f6568aec629abbb86f46979efa Reviewed-on: https://chromium-review.googlesource.com/c/1340827 Reviewed-by: Rohit Rao <rohitrao@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Commit-Queue: Rohit Rao <rohitrao@chromium.org> Cr-Commit-Position: refs/heads/master@{#616319} [modify] https://crrev.com/3d3f6bf113b71a9d0d2a259152581a5d65f76a22/ios/web/test/test_web_thread_bundle.cc [modify] https://crrev.com/3d3f6bf113b71a9d0d2a259152581a5d65f76a22/ios/web/web_thread_impl.cc [modify] https://crrev.com/3d3f6bf113b71a9d0d2a259152581a5d65f76a22/ios/web/web_thread_impl.h
,
Dec 13
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/be41fab665818d4184f70143c39f6caed80364fc commit be41fab665818d4184f70143c39f6caed80364fc Author: Javier Ernesto Flores Robles <javierrobles@chromium.org> Date: Thu Dec 13 17:27:17 2018 Revert "[ios] Store TaskRunners instead of threads in WebThreadImpl." This reverts commit 3d3f6bf113b71a9d0d2a259152581a5d65f76a22. Reason for revert: This broke ios_chrome_unittest, more info in crbug.com/914869 Original change's description: > [ios] Store TaskRunners instead of threads in WebThreadImpl. > > Track thread states explicitly instead of simply checking > whether a TaskRunner pointer exists or not. > > BUG=826465 > > Change-Id: Id0f84e7eedc6b9f6568aec629abbb86f46979efa > Reviewed-on: https://chromium-review.googlesource.com/c/1340827 > Reviewed-by: Rohit Rao <rohitrao@chromium.org> > Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> > Reviewed-by: Gabriel Charette <gab@chromium.org> > Commit-Queue: Rohit Rao <rohitrao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616319} TBR=rohitrao@chromium.org,gab@chromium.org,sdefresne@chromium.org Change-Id: I1297465d60f4b3b5a76882b5e723080c9376a7dc No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 826465 Reviewed-on: https://chromium-review.googlesource.com/c/1375890 Reviewed-by: Javier Ernesto Flores Robles <javierrobles@chromium.org> Commit-Queue: Javier Ernesto Flores Robles <javierrobles@chromium.org> Cr-Commit-Position: refs/heads/master@{#616348} [modify] https://crrev.com/be41fab665818d4184f70143c39f6caed80364fc/ios/web/test/test_web_thread_bundle.cc [modify] https://crrev.com/be41fab665818d4184f70143c39f6caed80364fc/ios/web/web_thread_impl.cc [modify] https://crrev.com/be41fab665818d4184f70143c39f6caed80364fc/ios/web/web_thread_impl.h
,
Dec 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e07395847275faf4e81181dce9e1474f5f85b4d1 commit e07395847275faf4e81181dce9e1474f5f85b4d1 Author: Rohit Rao <rohitrao@chromium.org> Date: Fri Dec 14 17:28:31 2018 Reland "[ios] Store TaskRunners instead of threads in WebThreadImpl." This is a reland of 3d3f6bf113b71a9d0d2a259152581a5d65f76a22 The call to ResetGlobalsForTesting() has been moved from TestWebThreadBundle to TestWebThread, as some unittests instantiate TestWebThreads directly. Original change's description: > [ios] Store TaskRunners instead of threads in WebThreadImpl. > > Track thread states explicitly instead of simply checking > whether a TaskRunner pointer exists or not. > > BUG=826465 > > Change-Id: Id0f84e7eedc6b9f6568aec629abbb86f46979efa > Reviewed-on: https://chromium-review.googlesource.com/c/1340827 > Reviewed-by: Rohit Rao <rohitrao@chromium.org> > Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> > Reviewed-by: Gabriel Charette <gab@chromium.org> > Commit-Queue: Rohit Rao <rohitrao@chromium.org> > Cr-Commit-Position: refs/heads/master@{#616319} Bug: 826465, 914869 Change-Id: I522b1bb46464a17ade84a20857ee7db2cb713a8d Reviewed-on: https://chromium-review.googlesource.com/c/1377320 Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Commit-Queue: Rohit Rao <rohitrao@chromium.org> Cr-Commit-Position: refs/heads/master@{#616723} [modify] https://crrev.com/e07395847275faf4e81181dce9e1474f5f85b4d1/ios/web/public/test/test_web_thread.h [modify] https://crrev.com/e07395847275faf4e81181dce9e1474f5f85b4d1/ios/web/test/test_web_thread.cc [modify] https://crrev.com/e07395847275faf4e81181dce9e1474f5f85b4d1/ios/web/web_thread_impl.cc [modify] https://crrev.com/e07395847275faf4e81181dce9e1474f5f85b4d1/ios/web/web_thread_impl.h
,
Dec 22
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/752eeba72e3136a4d81cfbcff56d6dffd1338ac0 commit 752eeba72e3136a4d81cfbcff56d6dffd1338ac0 Author: Rohit Rao <rohitrao@chromium.org> Date: Sat Dec 22 19:28:08 2018 Refactor WebThreadImpl, WebSubThread, and WebMainLoop. WebThreadImpl is now merely a scoped object that binds a provided SingleThreadTaskRunner to a WebThread::ID. It no longer subclasses base::Thread. That functionality has been moved to WebSubThread, which is effectively only used for the IO thread now. This is the //web implementation of https://chromium-review.googlesource.com/c/chromium/src/+/980793. See the description of that CL for more details. BUG=826465 Change-Id: I636051b154c4156bc32517d649081131f9bdd14d Reviewed-on: https://chromium-review.googlesource.com/c/1383140 Commit-Queue: Rohit Rao <rohitrao@chromium.org> Reviewed-by: Gabriel Charette <gab@chromium.org> Reviewed-by: Sylvain Defresne <sdefresne@chromium.org> Cr-Commit-Position: refs/heads/master@{#618761} [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/base/threading/thread_restrictions.h [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/components/io_thread/DEPS [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/components/io_thread/ios_io_thread_unittest.mm [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/app/web_main_loop.h [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/app/web_main_loop.mm [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/public/test/test_web_thread.h [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/public/web_thread.h [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/public/web_thread_delegate.h [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/test/test_web_thread.cc [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/web_sub_thread.cc [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/web_sub_thread.h [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/web_thread_impl.cc [modify] https://crrev.com/752eeba72e3136a4d81cfbcff56d6dffd1338ac0/ios/web/web_thread_impl.h |
||||
►
Sign in to add a comment |
||||
Comment 1 by bugdroid1@chromium.org
, Oct 31