Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2370753002:
Make TabLoader a client of memory coordinator (Closed)
|
Created:
4 years, 2 months ago by hajimehoshi Modified:
4 years, 1 month ago CC:
chromium-reviews Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionMake TabLoader a client of memory coordinator
When the memory coordinator is enabled, TabLoader becomes a client of
the memory coordinator instead of installing MemoryPressureListener.
Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8Uo/edit?ts=57d7968b#
BUG=639700
Committed: https://crrev.com/30b8b607a1d2ac63038383685d8eedf44acb0447
Cr-Commit-Position: refs/heads/master@{#427603}
Patch Set 1 #
Total comments: 8
Patch Set 2 : Address on reviews #Patch Set 3 : Add comments #
Total comments: 3
Patch Set 4 : Use MemoryCoordinatorProxy #
Depends on Patchset: Messages
Total messages: 45 (15 generated)
hajimehoshi@chromium.org changed reviewers: + bashi@chromium.org, chrisha@chromium.org, sky@chromium.org
PTAL
https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... File chrome/browser/sessions/tab_loader.cc (right): https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:276: void TabLoader::PurgeMemory() { PurgeMemory isn't the best name here, as it's not actually freeing up any memory. Rather, it's stopping further use of memory. Rename this StopLoadingTabs, and ShouldStopLoadingTabs?
https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... File chrome/browser/sessions/tab_loader.cc (right): https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:84: memory_state_(base::MemoryState::NORMAL), Should this be initialized to the real value? By that I mean if the memory coordinate client is enabled shouldn't this query for the current value? https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:244: if (base::FeatureList::IsEnabled(features::kMemoryCoordinator)) { no {} https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:276: void TabLoader::PurgeMemory() { On 2016/09/26 14:18:53, chrisha (slow) wrote: > PurgeMemory isn't the best name here, as it's not actually freeing up any > memory. Rather, it's stopping further use of memory. Rename this > StopLoadingTabs, and ShouldStopLoadingTabs? SGTM
Thank you! https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... File chrome/browser/sessions/tab_loader.cc (right): https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:84: memory_state_(base::MemoryState::NORMAL), On 2016/09/26 16:41:57, sky wrote: > Should this be initialized to the real value? By that I mean if the memory > coordinate client is enabled shouldn't this query for the current value? chrisha@: IIUC the initial value should always be NORMAL, but is this correct? https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:244: if (base::FeatureList::IsEnabled(features::kMemoryCoordinator)) { On 2016/09/26 16:41:57, sky wrote: > no {} Done. https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:276: void TabLoader::PurgeMemory() { On 2016/09/26 16:41:57, sky wrote: > On 2016/09/26 14:18:53, chrisha (slow) wrote: > > PurgeMemory isn't the best name here, as it's not actually freeing up any > > memory. Rather, it's stopping further use of memory. Rename this > > StopLoadingTabs, and ShouldStopLoadingTabs? > > SGTM Done.
https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... File chrome/browser/sessions/tab_loader.cc (right): https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... chrome/browser/sessions/tab_loader.cc:84: memory_state_(base::MemoryState::NORMAL), On 2016/09/27 05:59:16, hajimehoshi wrote: > On 2016/09/26 16:41:57, sky wrote: > > Should this be initialized to the real value? By that I mean if the memory > > coordinate client is enabled shouldn't this query for the current value? > > chrisha@: IIUC the initial value should always be NORMAL, but is this correct? Yes. Current plan is that the initial state is NORMAL.
On 2016/09/29 06:30:52, bashi1 wrote: > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > File chrome/browser/sessions/tab_loader.cc (right): > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > chrome/browser/sessions/tab_loader.cc:84: > memory_state_(base::MemoryState::NORMAL), > On 2016/09/27 05:59:16, hajimehoshi wrote: > > On 2016/09/26 16:41:57, sky wrote: > > > Should this be initialized to the real value? By that I mean if the memory > > > coordinate client is enabled shouldn't this query for the current value? > > > > chrisha@: IIUC the initial value should always be NORMAL, but is this correct? > > Yes. Current plan is that the initial state is NORMAL. For now, yes. But we could envision starting a background renderer at a lower priority. I think we need a mechanism for the browser MC or CMC to expose its knowledge of the current level via the interface in base, allowing MCCs to query the current level. This would prevent each of these implementations from having their own local cache of the state, and hardcoding assumed initial values. Thoughts?
On 2016/09/29 15:09:21, chrisha (slow) wrote: > On 2016/09/29 06:30:52, bashi1 wrote: > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > File chrome/browser/sessions/tab_loader.cc (right): > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > chrome/browser/sessions/tab_loader.cc:84: > > memory_state_(base::MemoryState::NORMAL), > > On 2016/09/27 05:59:16, hajimehoshi wrote: > > > On 2016/09/26 16:41:57, sky wrote: > > > > Should this be initialized to the real value? By that I mean if the memory > > > > coordinate client is enabled shouldn't this query for the current value? > > > > > > chrisha@: IIUC the initial value should always be NORMAL, but is this > correct? > > > > Yes. Current plan is that the initial state is NORMAL. > > For now, yes. But we could envision starting a background renderer at a lower > priority. I think we need a mechanism for the browser MC or CMC to expose its > knowledge of the current level via the interface in base, allowing MCCs to query > the current level. This would prevent each of these implementations from having > their own local cache of the state, and hardcoding assumed initial values. > Thoughts? Yes. That makes sense to me. I'll think about it. (TabLoader is in browser process and I think it would be reasonable to assume that the initial state of browse is NORMAL)
On 2016/09/29 22:58:38, bashi1 wrote: > On 2016/09/29 15:09:21, chrisha (slow) wrote: > > On 2016/09/29 06:30:52, bashi1 wrote: > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > File chrome/browser/sessions/tab_loader.cc (right): > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > chrome/browser/sessions/tab_loader.cc:84: > > > memory_state_(base::MemoryState::NORMAL), > > > On 2016/09/27 05:59:16, hajimehoshi wrote: > > > > On 2016/09/26 16:41:57, sky wrote: > > > > > Should this be initialized to the real value? By that I mean if the > memory > > > > > coordinate client is enabled shouldn't this query for the current value? > > > > > > > > chrisha@: IIUC the initial value should always be NORMAL, but is this > > correct? > > > > > > Yes. Current plan is that the initial state is NORMAL. > > > > For now, yes. But we could envision starting a background renderer at a lower > > priority. I think we need a mechanism for the browser MC or CMC to expose its > > knowledge of the current level via the interface in base, allowing MCCs to > query > > the current level. This would prevent each of these implementations from > having > > their own local cache of the state, and hardcoding assumed initial values. > > Thoughts? > > Yes. That makes sense to me. I'll think about it. > > (TabLoader is in browser process and I think it would be reasonable to assume > that the initial state of browse is NORMAL) Wouldn't it be simpler that the initial value of a MCC component is always NORMAL and if its actual state is not NORMAL, its OnMemoryStateChange is called as soon as possible?
Description was changed from ========== Make TabLoader a client of memory coordinator When the memory coordinator is enabled, TabLoader becomes a client of the memory coordinator instead of installing MemoryPressureListener. Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8... BUG=639700 ========== to ========== Make TabLoader a client of memory coordinator When the memory coordinator is enabled, TabLoader becomes a client of the memory coordinator instead of installing MemoryPressureListener. Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8... BUG=639700 ==========
hajimehoshi@chromium.org changed reviewers: + haraken@chromium.org
+haraken
On 2016/09/30 04:40:10, hajimehoshi wrote: > On 2016/09/29 22:58:38, bashi1 wrote: > > On 2016/09/29 15:09:21, chrisha (slow) wrote: > > > On 2016/09/29 06:30:52, bashi1 wrote: > > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > > File chrome/browser/sessions/tab_loader.cc (right): > > > > > > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > > chrome/browser/sessions/tab_loader.cc:84: > > > > memory_state_(base::MemoryState::NORMAL), > > > > On 2016/09/27 05:59:16, hajimehoshi wrote: > > > > > On 2016/09/26 16:41:57, sky wrote: > > > > > > Should this be initialized to the real value? By that I mean if the > > memory > > > > > > coordinate client is enabled shouldn't this query for the current > value? > > > > > > > > > > chrisha@: IIUC the initial value should always be NORMAL, but is this > > > correct? > > > > > > > > Yes. Current plan is that the initial state is NORMAL. > > > > > > For now, yes. But we could envision starting a background renderer at a > lower > > > priority. I think we need a mechanism for the browser MC or CMC to expose > its > > > knowledge of the current level via the interface in base, allowing MCCs to > > query > > > the current level. This would prevent each of these implementations from > > having > > > their own local cache of the state, and hardcoding assumed initial values. > > > Thoughts? > > > > Yes. That makes sense to me. I'll think about it. > > > > (TabLoader is in browser process and I think it would be reasonable to assume > > that the initial state of browse is NORMAL) > > Wouldn't it be simpler that the initial value of a MCC component is always > NORMAL and if its actual state is not NORMAL, its OnMemoryStateChange is called > as soon as possible? I'll defer the decision to chrisha@ and bashi@. However, I think we can add a TODO and land this CL.
On 2016/09/30 05:27:21, haraken wrote: > On 2016/09/30 04:40:10, hajimehoshi wrote: > > On 2016/09/29 22:58:38, bashi1 wrote: > > > On 2016/09/29 15:09:21, chrisha (slow) wrote: > > > > On 2016/09/29 06:30:52, bashi1 wrote: > > > > > > > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > > > File chrome/browser/sessions/tab_loader.cc (right): > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > > > chrome/browser/sessions/tab_loader.cc:84: > > > > > memory_state_(base::MemoryState::NORMAL), > > > > > On 2016/09/27 05:59:16, hajimehoshi wrote: > > > > > > On 2016/09/26 16:41:57, sky wrote: > > > > > > > Should this be initialized to the real value? By that I mean if the > > > memory > > > > > > > coordinate client is enabled shouldn't this query for the current > > value? > > > > > > > > > > > > chrisha@: IIUC the initial value should always be NORMAL, but is this > > > > correct? > > > > > > > > > > Yes. Current plan is that the initial state is NORMAL. > > > > > > > > For now, yes. But we could envision starting a background renderer at a > > lower > > > > priority. I think we need a mechanism for the browser MC or CMC to expose > > its > > > > knowledge of the current level via the interface in base, allowing MCCs to > > > query > > > > the current level. This would prevent each of these implementations from > > > having > > > > their own local cache of the state, and hardcoding assumed initial values. > > > > Thoughts? > > > > > > Yes. That makes sense to me. I'll think about it. > > > > > > (TabLoader is in browser process and I think it would be reasonable to > assume > > > that the initial state of browse is NORMAL) > > > > Wouldn't it be simpler that the initial value of a MCC component is always > > NORMAL and if its actual state is not NORMAL, its OnMemoryStateChange is > called > > as soon as possible? > > I'll defer the decision to chrisha@ and bashi@. However, I think we can add a > TODO and land this CL. Ah, I missed the benefit of avoiding to have the state cache in each component. I'll follow your opinions too.
On 2016/09/30 05:37:51, hajimehoshi wrote: > On 2016/09/30 05:27:21, haraken wrote: > > On 2016/09/30 04:40:10, hajimehoshi wrote: > > > On 2016/09/29 22:58:38, bashi1 wrote: > > > > On 2016/09/29 15:09:21, chrisha (slow) wrote: > > > > > On 2016/09/29 06:30:52, bashi1 wrote: > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > > > > File chrome/browser/sessions/tab_loader.cc (right): > > > > > > > > > > > > > > > > > > > > > > > > > > > https://codereview.chromium.org/2370753002/diff/1/chrome/browser/sessions/tab... > > > > > > chrome/browser/sessions/tab_loader.cc:84: > > > > > > memory_state_(base::MemoryState::NORMAL), > > > > > > On 2016/09/27 05:59:16, hajimehoshi wrote: > > > > > > > On 2016/09/26 16:41:57, sky wrote: > > > > > > > > Should this be initialized to the real value? By that I mean if > the > > > > memory > > > > > > > > coordinate client is enabled shouldn't this query for the current > > > value? > > > > > > > > > > > > > > chrisha@: IIUC the initial value should always be NORMAL, but is > this > > > > > correct? > > > > > > > > > > > > Yes. Current plan is that the initial state is NORMAL. > > > > > > > > > > For now, yes. But we could envision starting a background renderer at a > > > lower > > > > > priority. I think we need a mechanism for the browser MC or CMC to > expose > > > its > > > > > knowledge of the current level via the interface in base, allowing MCCs > to > > > > query > > > > > the current level. This would prevent each of these implementations from > > > > having > > > > > their own local cache of the state, and hardcoding assumed initial > values. > > > > > Thoughts? > > > > > > > > Yes. That makes sense to me. I'll think about it. > > > > > > > > (TabLoader is in browser process and I think it would be reasonable to > > assume > > > > that the initial state of browse is NORMAL) > > > > > > Wouldn't it be simpler that the initial value of a MCC component is always > > > NORMAL and if its actual state is not NORMAL, its OnMemoryStateChange is > > called > > > as soon as possible? > > > > I'll defer the decision to chrisha@ and bashi@. However, I think we can add a > > TODO and land this CL. > > Ah, I missed the benefit of avoiding to have the state cache in each component. > I'll follow your opinions too. I think your proposal may be a reasonable option when we change a signature of OnMemoryStateChange like: void OnMemoryStateChange(MemoryState prev, MemoryState current); One major downside of adding a state getter function in base/ is that we need to do dependency injection from content to base. At this point I'm not sure which one is better. Let me think it over.
lgtm with a TODO to move this to MC
On 2016/10/07 13:04:00, chrisha (slow) wrote: > lgtm with a TODO to move this to MC Thanks, done. sky@: PTAL
The CQ bit was checked by hajimehoshi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... File chrome/browser/sessions/tab_loader.h (right): https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... chrome/browser/sessions/tab_loader.h:130: base::MemoryState memory_state_; Is it hard to implement the API to get MemoryState? It's unfortunate that you're adding memory_state_ to a bunch of places. If it's easy to implement the API, it's better to land the API first.
On 2016/10/11 08:36:23, haraken wrote: > https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... > File chrome/browser/sessions/tab_loader.h (right): > > https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... > chrome/browser/sessions/tab_loader.h:130: base::MemoryState memory_state_; > > Is it hard to implement the API to get MemoryState? > > It's unfortunate that you're adding memory_state_ to a bunch of places. If it's > easy to implement the API, it's better to land the API first. It's not hard but it looks like there is no implementation to notify browser components of their memory states, so we'd need to design and implement that first. As this CL is not urgent, I'll start to do that.
https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... File chrome/browser/sessions/tab_loader.h (right): https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... chrome/browser/sessions/tab_loader.h:130: base::MemoryState memory_state_; On 2016/10/11 08:36:23, haraken wrote: > > Is it hard to implement the API to get MemoryState? > > It's unfortunate that you're adding memory_state_ to a bunch of places. If it's > easy to implement the API, it's better to land the API first. +1
Thank you! https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... File chrome/browser/sessions/tab_loader.h (right): https://codereview.chromium.org/2370753002/diff/40001/chrome/browser/sessions... chrome/browser/sessions/tab_loader.h:130: base::MemoryState memory_state_; On 2016/10/11 17:47:42, sky wrote: > On 2016/10/11 08:36:23, haraken wrote: > > > > Is it hard to implement the API to get MemoryState? > > > > It's unfortunate that you're adding memory_state_ to a bunch of places. If > it's > > easy to implement the API, it's better to land the API first. > > +1 Done.
The CQ bit was checked by hajimehoshi@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Is there test coverage of this? Can existing tests be made to trigger the new path?
On 2016/10/25 02:57:14, sky wrote: > Is there test coverage of this? Can existing tests be made to trigger the new > path? There is not, but should be a test for this. +chrisha, +haraken My plan is: 1. Add base::MemoryCoordinatorProxy::SetCurrentMemoryStateForTesting 2. Add TabLoaderBrowserTest using this The new setter can be another place (e.g. content/public) but regarding /net, I think the setter should be in /base. What do you think?
On 2016/10/25 08:32:51, hajimehoshi wrote: > On 2016/10/25 02:57:14, sky wrote: > > Is there test coverage of this? Can existing tests be made to trigger the new > > path? > > There is not, but should be a test for this. > > +chrisha, +haraken > > My plan is: > > 1. Add base::MemoryCoordinatorProxy::SetCurrentMemoryStateForTesting > 2. Add TabLoaderBrowserTest using this > > The new setter can be another place (e.g. content/public) but regarding /net, I > think the setter should be in /base. > > What do you think? +1 to put the setter in base/ if needed. There could be other components which want to use the setter for testing (e.g. cc).
On 2016/10/25 08:53:01, bashi1 wrote: > On 2016/10/25 08:32:51, hajimehoshi wrote: > > On 2016/10/25 02:57:14, sky wrote: > > > Is there test coverage of this? Can existing tests be made to trigger the > new > > > path? > > > > There is not, but should be a test for this. > > > > +chrisha, +haraken > > > > My plan is: > > > > 1. Add base::MemoryCoordinatorProxy::SetCurrentMemoryStateForTesting > > 2. Add TabLoaderBrowserTest using this > > > > The new setter can be another place (e.g. content/public) but regarding /net, > I > > think the setter should be in /base. > > > > What do you think? > > +1 to put the setter in base/ if needed. There could be other components which > want to use the setter for testing (e.g. cc). Agreed.
LGTM - I would prefer the test first, but I don't want to hold you up any longer.
The CQ bit was checked by hajimehoshi@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrisha@chromium.org Link to the patchset: https://codereview.chromium.org/2370753002/#ps60001 (title: "Use MemoryCoordinatorProxy")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Make TabLoader a client of memory coordinator When the memory coordinator is enabled, TabLoader becomes a client of the memory coordinator instead of installing MemoryPressureListener. Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8... BUG=639700 ========== to ========== Make TabLoader a client of memory coordinator When the memory coordinator is enabled, TabLoader becomes a client of the memory coordinator instead of installing MemoryPressureListener. Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8... BUG=639700 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Make TabLoader a client of memory coordinator When the memory coordinator is enabled, TabLoader becomes a client of the memory coordinator instead of installing MemoryPressureListener. Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8... BUG=639700 ========== to ========== Make TabLoader a client of memory coordinator When the memory coordinator is enabled, TabLoader becomes a client of the memory coordinator instead of installing MemoryPressureListener. Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8... BUG=639700 Committed: https://crrev.com/30b8b607a1d2ac63038383685d8eedf44acb0447 Cr-Commit-Position: refs/heads/master@{#427603} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/30b8b607a1d2ac63038383685d8eedf44acb0447 Cr-Commit-Position: refs/heads/master@{#427603}
Message was sent while issue was closed.
sky: Hmm, I have struggled with implementing tests, but am stuck with it. For unit tests, TabLoader is not a self-contained class and it seems very difficult to write this. On the other hand, for browser tests, I have no idea how to access a TabLoader and how to initialize the flag --memory-coordinator before the browser main loop starts. In the first place, there seems no tests for TabLoader in the code base. Would you give me an advice how to add a test for TabLoader?
Message was sent while issue was closed.
TabLoader was refactored out of session restore code. TabLoader is indirectly tested by various session restore code. It seems like you should be able to write a test uses TabLoader directly, e.g. do what session restore does: call TabLoader::RestoreTabs. For the case you care about, changing memory pressure, it seems as though you should be able to call OnMemoryStateChange() and make sure the right thing happens. -Scott On Mon, Oct 31, 2016 at 2:19 AM, <hajimehoshi@chromium.org> wrote: > sky: Hmm, I have struggled with implementing tests, but am stuck with it. > > For unit tests, TabLoader is not a self-contained class and it seems very > difficult to write this. On the other hand, for browser tests, I have no > idea > how to access a TabLoader and how to initialize the flag > --memory-coordinator > before the browser main loop starts. > > In the first place, there seems no tests for TabLoader in the code base. > Would > you give me an advice how to add a test for TabLoader? > > https://codereview.chromium.org/2370753002/ -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
