|
|
Chromium Code Reviews|
Created:
8 years, 3 months ago by simonhong_ Modified:
8 years, 3 months ago CC:
chromium-reviews Base URL:
http://git.chromium.org/chromium/src.git@new Visibility:
Public. |
DescriptionWhen chrome is in low memory condition, it also discards selected tab.
Selected tab should not be discarded.
BUG=151446
TEST=OomPriorityManagerTest.OomPriorityManagerBasics
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=158333
Patch Set 1 #
Total comments: 4
Patch Set 2 : Modify and Add Unit Tests #Patch Set 3 : #
Total comments: 2
Patch Set 4 : Disable discard selected tab and its unit test #
Total comments: 2
Patch Set 5 : handle active tab #
Messages
Total messages: 20 (0 generated)
Dear sky, Please review this patch. Thank you.
https://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_st... File chrome/browser/ui/tabs/tab_strip_model.cc (right): https://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_st... chrome/browser/ui/tabs/tab_strip_model.cc:221: // Only mark tabs that are not selected. When selected tab is discarded, it is Write a test for this.
http://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_str... File chrome/browser/ui/tabs/tab_strip_model.cc (right): http://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_str... chrome/browser/ui/tabs/tab_strip_model.cc:221: // Only mark tabs that are not selected. When selected tab is discarded, it is This can be verified by OomProirotyManager::DiscardTab(). In this situation, TabStripModel::DiscardTabContentsAt() is not called because last tab is marked true. Does I make test in tab_strip_model_unittest.cc or oom_priority_manager_unittest? On 2012/09/19 17:06:59, sky wrote: > Write a test for this.
http://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_str... File chrome/browser/ui/tabs/tab_strip_model.cc (right): http://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_str... chrome/browser/ui/tabs/tab_strip_model.cc:221: // Only mark tabs that are not selected. When selected tab is discarded, it is On 2012/09/20 10:26:29, Simon Hong wrote: > This can be verified by OomProirotyManager::DiscardTab(). Can't you call what OOMPriorityManager is invoking? > In this situation, TabStripModel::DiscardTabContentsAt() is not called because > last tab is marked true. > Does I make test in tab_strip_model_unittest.cc or > oom_priority_manager_unittest? tab_strip_model_unittest since that where the fix is going. > On 2012/09/19 17:06:59, sky wrote: > > Write a test for this. >
https://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_st... File chrome/browser/ui/tabs/tab_strip_model.cc (right): https://codereview.chromium.org/10937028/diff/1/chrome/browser/ui/tabs/tab_st... chrome/browser/ui/tabs/tab_strip_model.cc:221: // Only mark tabs that are not selected. When selected tab is discarded, it is Modified tab_strip_model_unittest. And Added some test to oom_priority_mamanger_browsertest. On 2012/09/20 15:24:08, sky wrote: > On 2012/09/20 10:26:29, Simon Hong wrote: > > This can be verified by OomProirotyManager::DiscardTab(). > > Can't you call what OOMPriorityManager is invoking? > > > In this situation, TabStripModel::DiscardTabContentsAt() is not called because > > last tab is marked true. > > Does I make test in tab_strip_model_unittest.cc or > > oom_priority_manager_unittest? > > tab_strip_model_unittest since that where the fix is going. > > > On 2012/09/19 17:06:59, sky wrote: > > > Write a test for this. > > >
https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... File chrome/browser/ui/tabs/tab_strip_model.cc (right): https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... chrome/browser/ui/tabs/tab_strip_model.cc:224: if (!IsTabSelected(index)) { I don't understand this. What is triggering the load again? Should we not discard selected tabs at all?
https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... File chrome/browser/ui/tabs/tab_strip_model.cc (right): https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... chrome/browser/ui/tabs/tab_strip_model.cc:224: if (!IsTabSelected(index)) { I'll explain problematic scenario according to this. There are three opened tabs, tab1, tab2 and tab3. tab3 is active tab. When low memory condition is occured, tab1 and tab2 is discarded because tab3 is active tab. If tab3 eats a lot of system memory, oom priority manager also discards tab3. Now, all of tabs are marked as discarded. When active tab is changed, ActiveTabChaged() is called. In Browser::ActiveTabChanged, active tab is reloaded again. Despite tab3 is opened, its discarded value is still true. If users open new tab and it also heavy web page, oom priority manager should discard tab3 to make more free memory. But tab3 cannot be discarded because its discarded status is true. I think updating discared status to false when tab3 is reloaded agin is more better way. On 2012/09/20 19:31:09, sky wrote: > I don't understand this. What is triggering the load again? Should we not > discard selected tabs at all?
On 2012/09/21 00:16:37, Simon Hong wrote: > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > File chrome/browser/ui/tabs/tab_strip_model.cc (right): > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > chrome/browser/ui/tabs/tab_strip_model.cc:224: if (!IsTabSelected(index)) { > I'll explain problematic scenario according to this. > There are three opened tabs, tab1, tab2 and tab3. > tab3 is active tab. > When low memory condition is occured, tab1 and tab2 is discarded because tab3 is > active tab. > If tab3 eats a lot of system memory, oom priority manager also discards tab3. > Now, all of tabs are marked as discarded. > When active tab is changed, ActiveTabChaged() is called. > In Browser::ActiveTabChanged, active tab is reloaded again. > Despite tab3 is opened, its discarded value is still true. > If users open new tab and it also heavy web page, oom priority manager should > discard tab3 to make more free memory. > But tab3 cannot be discarded because its discarded status is true. > > I think updating discared status to false when tab3 is reloaded agin is more > better way. > On 2012/09/20 19:31:09, sky wrote: > > I don't understand this. What is triggering the load again? Should we not > > discard selected tabs at all? Or, How about don't discard active tab? When active tab is discarded, it is reloaded again.
On Thu, Sep 20, 2012 at 5:16 PM, <simon.hong81@gmail.com> wrote: > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > File chrome/browser/ui/tabs/tab_strip_model.cc (right): > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > chrome/browser/ui/tabs/tab_strip_model.cc:224: if > (!IsTabSelected(index)) { > I'll explain problematic scenario according to this. > There are three opened tabs, tab1, tab2 and tab3. > tab3 is active tab. > When low memory condition is occured, tab1 and tab2 is discarded because > tab3 is active tab. > If tab3 eats a lot of system memory, oom priority manager also discards > tab3. > Now, all of tabs are marked as discarded. > When active tab is changed, ActiveTabChaged() is called. > In Browser::ActiveTabChanged, active tab is reloaded again. So, to make sure I understand things at this point all tabs have been discarded and 3 is active. Right? If the user then clicks on 1 we trigger loading 3 and 1? That's wrong. We should just load 1. -Scott > Despite tab3 is opened, its discarded value is still true. > If users open new tab and it also heavy web page, oom priority manager > should discard tab3 to make more free memory. > But tab3 cannot be discarded because its discarded status is true. > > I think updating discared status to false when tab3 is reloaded agin is > more better way. > > On 2012/09/20 19:31:09, sky wrote: >> >> I don't understand this. What is triggering the load again? Should we > > not >> >> discard selected tabs at all? > > > https://codereview.chromium.org/10937028/
2012. 9. 22. 오전 1:18에 "Scott Violet" <sky@chromium.org>님이 작성: > > On Thu, Sep 20, 2012 at 5:16 PM, <simon.hong81@gmail.com> wrote: > > > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > > File chrome/browser/ui/tabs/tab_strip_model.cc (right): > > > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > > chrome/browser/ui/tabs/tab_strip_model.cc:224: if > > (!IsTabSelected(index)) { > > I'll explain problematic scenario according to this. > > There are three opened tabs, tab1, tab2 and tab3. > > tab3 is active tab. > > When low memory condition is occured, tab1 and tab2 is discarded because > > tab3 is active tab. > > If tab3 eats a lot of system memory, oom priority manager also discards > > tab3. > > Now, all of tabs are marked as discarded. > > When active tab is changed, ActiveTabChaged() is called. > > In Browser::ActiveTabChanged, active tab is reloaded again. > > So, to make sure I understand things at this point all tabs have been > discarded and 3 is active. Right? If the user then clicks on 1 we > trigger loading 3 and 1? That's wrong. We should just load 1. Yes, when user clicks tab 1, only tab 1 is loaded. And tab 1's discarded status is changed to false. But tab 3's discarded status is still true. Discarded tab's status is changed when it becomes activated tab. When active tab is discarded, it is reloaded but its discarded status not changed to false. I think this is problem. > -Scott > > > Despite tab3 is opened, its discarded value is still true. > > If users open new tab and it also heavy web page, oom priority manager > > should discard tab3 to make more free memory. > > But tab3 cannot be discarded because its discarded status is true. > > > > I think updating discared status to false when tab3 is reloaded agin is > > more better way. > > > > On 2012/09/20 19:31:09, sky wrote: > >> > >> I don't understand this. What is triggering the load again? Should we > > > > not > >> > >> discard selected tabs at all? > > > > > > https://codereview.chromium.org/10937028/
On Fri, Sep 21, 2012 at 4:01 PM, Simon Hong <simon.hong81@gmail.com> wrote: > > 2012. 9. 22. 오전 1:18에 "Scott Violet" <sky@chromium.org>님이 작성: > > >> >> On Thu, Sep 20, 2012 at 5:16 PM, <simon.hong81@gmail.com> wrote: >> > >> > >> > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... >> > File chrome/browser/ui/tabs/tab_strip_model.cc (right): >> > >> > >> > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... >> > chrome/browser/ui/tabs/tab_strip_model.cc:224: if >> > (!IsTabSelected(index)) { >> > I'll explain problematic scenario according to this. >> > There are three opened tabs, tab1, tab2 and tab3. >> > tab3 is active tab. >> > When low memory condition is occured, tab1 and tab2 is discarded because >> > tab3 is active tab. >> > If tab3 eats a lot of system memory, oom priority manager also discards >> > tab3. >> > Now, all of tabs are marked as discarded. >> > When active tab is changed, ActiveTabChaged() is called. >> > In Browser::ActiveTabChanged, active tab is reloaded again. >> >> So, to make sure I understand things at this point all tabs have been >> discarded and 3 is active. Right? If the user then clicks on 1 we >> trigger loading 3 and 1? That's wrong. We should just load 1. > > Yes, when user clicks tab 1, only tab 1 is loaded. And tab 1's discarded > status is changed to false. > But tab 3's discarded status is still true. Is tab3 ever discarded? What triggers reloading it? -Scott > Discarded tab's status is changed when it becomes activated tab. > When active tab is discarded, it is reloaded but its discarded status not > changed to false. > I think this is problem. > > >> -Scott >> >> > Despite tab3 is opened, its discarded value is still true. >> > If users open new tab and it also heavy web page, oom priority manager >> > should discard tab3 to make more free memory. >> > But tab3 cannot be discarded because its discarded status is true. >> > >> > I think updating discared status to false when tab3 is reloaded agin is >> > more better way. >> > >> > On 2012/09/20 19:31:09, sky wrote: >> >> >> >> I don't understand this. What is triggering the load again? Should we >> > >> > not >> >> >> >> discard selected tabs at all? >> > >> > >> > https://codereview.chromium.org/10937028/
On Sat, Sep 22, 2012 at 8:12 AM, Scott Violet <sky@chromium.org> wrote: > On Fri, Sep 21, 2012 at 4:01 PM, Simon Hong <simon.hong81@gmail.com> > wrote: > > > > 2012. 9. 22. 오전 1:18에 "Scott Violet" <sky@chromium.org>님이 작성: > > > > > >> > >> On Thu, Sep 20, 2012 at 5:16 PM, <simon.hong81@gmail.com> wrote: > >> > > >> > > >> > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > >> > File chrome/browser/ui/tabs/tab_strip_model.cc (right): > >> > > >> > > >> > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > >> > chrome/browser/ui/tabs/tab_strip_model.cc:224: if > >> > (!IsTabSelected(index)) { > >> > I'll explain problematic scenario according to this. > >> > There are three opened tabs, tab1, tab2 and tab3. > >> > tab3 is active tab. > >> > When low memory condition is occured, tab1 and tab2 is discarded > because > >> > tab3 is active tab. > >> > If tab3 eats a lot of system memory, oom priority manager also > discards > >> > tab3. > >> > Now, all of tabs are marked as discarded. > >> > When active tab is changed, ActiveTabChaged() is called. > >> > In Browser::ActiveTabChanged, active tab is reloaded again. > >> > >> So, to make sure I understand things at this point all tabs have been > >> discarded and 3 is active. Right? If the user then clicks on 1 we > >> trigger loading 3 and 1? That's wrong. We should just load 1. > > > > Yes, when user clicks tab 1, only tab 1 is loaded. And tab 1's discarded > > status is changed to false. > > But tab 3's discarded status is still true. > > Is tab3 ever discarded? What triggers reloading it? > TabStripModel::ReplaceTabContentsAt() calls ActiveTabChanged() of TabStripModelObserver. In the Browser::TabReplacedAt(), discarded tab is reloaded. // Discarded tabs always get reloaded. if (!did_reload && tab_strip_model_->IsTabDiscarded(index)) { LOG(WARNING) << "Reloading discarded tab at " << index; static int reload_count = 0; UMA_HISTOGRAM_CUSTOM_COUNTS( "Tabs.Discard.ReloadCount", ++reload_count, 1, 1000, 50); chrome::Reload(this, CURRENT_TAB); } > > -Scott > > > Discarded tab's status is changed when it becomes activated tab. > > When active tab is discarded, it is reloaded but its discarded status not > > changed to false. > > I think this is problem. > > > > > >> -Scott > >> > >> > Despite tab3 is opened, its discarded value is still true. > >> > If users open new tab and it also heavy web page, oom priority manager > >> > should discard tab3 to make more free memory. > >> > But tab3 cannot be discarded because its discarded status is true. > >> > > >> > I think updating discared status to false when tab3 is reloaded agin > is > >> > more better way. > >> > > >> > On 2012/09/20 19:31:09, sky wrote: > >> >> > >> >> I don't understand this. What is triggering the load again? Should we > >> > > >> > not > >> >> > >> >> discard selected tabs at all? > >> > > >> > > >> > https://codereview.chromium.org/10937028/ >
The right fix is either not to do that, or ignore requests to discard the active tab. I tend to think ignoring requests to discard the active tab is the right thing to do since discarding the active tab leaves a very broken UI. -Scott On Fri, Sep 21, 2012 at 4:22 PM, Simon Hong <simon.hong81@gmail.com> wrote: > > > On Sat, Sep 22, 2012 at 8:12 AM, Scott Violet <sky@chromium.org> wrote: >> >> On Fri, Sep 21, 2012 at 4:01 PM, Simon Hong <simon.hong81@gmail.com> >> wrote: >> > >> > 2012. 9. 22. 오전 1:18에 "Scott Violet" <sky@chromium.org>님이 작성: >> > >> > >> >> >> >> On Thu, Sep 20, 2012 at 5:16 PM, <simon.hong81@gmail.com> wrote: >> >> > >> >> > >> >> > >> >> > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... >> >> > File chrome/browser/ui/tabs/tab_strip_model.cc (right): >> >> > >> >> > >> >> > >> >> > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... >> >> > chrome/browser/ui/tabs/tab_strip_model.cc:224: if >> >> > (!IsTabSelected(index)) { >> >> > I'll explain problematic scenario according to this. >> >> > There are three opened tabs, tab1, tab2 and tab3. >> >> > tab3 is active tab. >> >> > When low memory condition is occured, tab1 and tab2 is discarded >> >> > because >> >> > tab3 is active tab. >> >> > If tab3 eats a lot of system memory, oom priority manager also >> >> > discards >> >> > tab3. >> >> > Now, all of tabs are marked as discarded. >> >> > When active tab is changed, ActiveTabChaged() is called. >> >> > In Browser::ActiveTabChanged, active tab is reloaded again. >> >> >> >> So, to make sure I understand things at this point all tabs have been >> >> discarded and 3 is active. Right? If the user then clicks on 1 we >> >> trigger loading 3 and 1? That's wrong. We should just load 1. >> > >> > Yes, when user clicks tab 1, only tab 1 is loaded. And tab 1's discarded >> > status is changed to false. >> > But tab 3's discarded status is still true. >> >> Is tab3 ever discarded? What triggers reloading it? > > > TabStripModel::ReplaceTabContentsAt() calls ActiveTabChanged() of > TabStripModelObserver. > In the Browser::TabReplacedAt(), discarded tab is reloaded. > > // Discarded tabs always get reloaded. > if (!did_reload && tab_strip_model_->IsTabDiscarded(index)) { > LOG(WARNING) << "Reloading discarded tab at " << index; > static int reload_count = 0; > UMA_HISTOGRAM_CUSTOM_COUNTS( > "Tabs.Discard.ReloadCount", ++reload_count, 1, 1000, 50); > chrome::Reload(this, CURRENT_TAB); > } > > >> >> >> -Scott >> >> > Discarded tab's status is changed when it becomes activated tab. >> > When active tab is discarded, it is reloaded but its discarded status >> > not >> > changed to false. >> > I think this is problem. >> > >> > >> >> -Scott >> >> >> >> > Despite tab3 is opened, its discarded value is still true. >> >> > If users open new tab and it also heavy web page, oom priority >> >> > manager >> >> > should discard tab3 to make more free memory. >> >> > But tab3 cannot be discarded because its discarded status is true. >> >> > >> >> > I think updating discared status to false when tab3 is reloaded agin >> >> > is >> >> > more better way. >> >> > >> >> > On 2012/09/20 19:31:09, sky wrote: >> >> >> >> >> >> I don't understand this. What is triggering the load again? Should >> >> >> we >> >> > >> >> > not >> >> >> >> >> >> discard selected tabs at all? >> >> > >> >> > >> >> > https://codereview.chromium.org/10937028/ > >
I also think that disabling discarding the active tab is better. I'll make a patch that soon! On Sat, Sep 22, 2012 at 8:33 AM, Scott Violet <sky@chromium.org> wrote: > The right fix is either not to do that, or ignore requests to discard > the active tab. I tend to think ignoring requests to discard the > active tab is the right thing to do since discarding the active tab > leaves a very broken UI. > > -Scott > > On Fri, Sep 21, 2012 at 4:22 PM, Simon Hong <simon.hong81@gmail.com> > wrote: > > > > > > On Sat, Sep 22, 2012 at 8:12 AM, Scott Violet <sky@chromium.org> wrote: > >> > >> On Fri, Sep 21, 2012 at 4:01 PM, Simon Hong <simon.hong81@gmail.com> > >> wrote: > >> > > >> > 2012. 9. 22. 오전 1:18에 "Scott Violet" <sky@chromium.org>님이 작성: > >> > > >> > > >> >> > >> >> On Thu, Sep 20, 2012 at 5:16 PM, <simon.hong81@gmail.com> wrote: > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > >> >> > File chrome/browser/ui/tabs/tab_strip_model.cc (right): > >> >> > > >> >> > > >> >> > > >> >> > > https://codereview.chromium.org/10937028/diff/4002/chrome/browser/ui/tabs/tab... > >> >> > chrome/browser/ui/tabs/tab_strip_model.cc:224: if > >> >> > (!IsTabSelected(index)) { > >> >> > I'll explain problematic scenario according to this. > >> >> > There are three opened tabs, tab1, tab2 and tab3. > >> >> > tab3 is active tab. > >> >> > When low memory condition is occured, tab1 and tab2 is discarded > >> >> > because > >> >> > tab3 is active tab. > >> >> > If tab3 eats a lot of system memory, oom priority manager also > >> >> > discards > >> >> > tab3. > >> >> > Now, all of tabs are marked as discarded. > >> >> > When active tab is changed, ActiveTabChaged() is called. > >> >> > In Browser::ActiveTabChanged, active tab is reloaded again. > >> >> > >> >> So, to make sure I understand things at this point all tabs have been > >> >> discarded and 3 is active. Right? If the user then clicks on 1 we > >> >> trigger loading 3 and 1? That's wrong. We should just load 1. > >> > > >> > Yes, when user clicks tab 1, only tab 1 is loaded. And tab 1's > discarded > >> > status is changed to false. > >> > But tab 3's discarded status is still true. > >> > >> Is tab3 ever discarded? What triggers reloading it? > > > > > > TabStripModel::ReplaceTabContentsAt() calls ActiveTabChanged() of > > TabStripModelObserver. > > In the Browser::TabReplacedAt(), discarded tab is reloaded. > > > > // Discarded tabs always get reloaded. > > if (!did_reload && tab_strip_model_->IsTabDiscarded(index)) { > > LOG(WARNING) << "Reloading discarded tab at " << index; > > static int reload_count = 0; > > UMA_HISTOGRAM_CUSTOM_COUNTS( > > "Tabs.Discard.ReloadCount", ++reload_count, 1, 1000, 50); > > chrome::Reload(this, CURRENT_TAB); > > } > > > > > >> > >> > >> -Scott > >> > >> > Discarded tab's status is changed when it becomes activated tab. > >> > When active tab is discarded, it is reloaded but its discarded status > >> > not > >> > changed to false. > >> > I think this is problem. > >> > > >> > > >> >> -Scott > >> >> > >> >> > Despite tab3 is opened, its discarded value is still true. > >> >> > If users open new tab and it also heavy web page, oom priority > >> >> > manager > >> >> > should discard tab3 to make more free memory. > >> >> > But tab3 cannot be discarded because its discarded status is true. > >> >> > > >> >> > I think updating discared status to false when tab3 is reloaded > agin > >> >> > is > >> >> > more better way. > >> >> > > >> >> > On 2012/09/20 19:31:09, sky wrote: > >> >> >> > >> >> >> I don't understand this. What is triggering the load again? Should > >> >> >> we > >> >> > > >> >> > not > >> >> >> > >> >> >> discard selected tabs at all? > >> >> > > >> >> > > >> >> > https://codereview.chromium.org/10937028/ > > > > >
Dear Scott, New patch and unit test are uploaded. Please review this patch. Thank you.
https://codereview.chromium.org/10937028/diff/10003/chrome/browser/chromeos/o... File chrome/browser/chromeos/oom_priority_manager.cc (right): https://codereview.chromium.org/10937028/diff/10003/chrome/browser/chromeos/o... chrome/browser/chromeos/oom_priority_manager.cc:234: if (model->IsTabDiscarded(idx) || model->IsTabSelected(idx)) Is it the selected tab, or the active tab we care about? Also, can we make DiscardTabContentsAt in the model do nothing if invoked for the active tab.
https://codereview.chromium.org/10937028/diff/10003/chrome/browser/chromeos/o... File chrome/browser/chromeos/oom_priority_manager.cc (right): https://codereview.chromium.org/10937028/diff/10003/chrome/browser/chromeos/o... chrome/browser/chromeos/oom_priority_manager.cc:234: if (model->IsTabDiscarded(idx) || model->IsTabSelected(idx)) On 2012/09/24 13:56:11, sky wrote: > Is it the selected tab, or the active tab we care about? > > Also, can we make DiscardTabContentsAt in the model do nothing if invoked for > the active tab. Done.
LGTM
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/simon.hong81@gmail.com/10937028/15001
Change committed as 158333 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
