Reduce the in-memory blob storgae limit on Android

Reduce the in-memory blob storgae limit on Android

BUG=715859
Review-Url: https://codereview.chromium.org/2855943003
Cr-Commit-Position: refs/heads/master@{#469550}
Committed: https://chromium.googlesource.com/chromium/src/+/81a539c78aaf463a2aeb90c10366e586f6d85ab5

[ssid, 2017-05-02 21:32:21]

ssid@chromium.org changed reviewers:
+ dmurph@chromium.org

[ssid, 2017-05-02 21:32:22]

+Daniel, wdyt?

[dmurph, 2017-05-02 22:02:20]

On 2017/05/02 21:32:22, ssid wrote:
> +Daniel, wdyt?

One of the things that guided us initially was to keep the effective max storage
around 500MB. This lowers that to 410MB for android minimal - can we slightly
increase the android disk storage to go up a little? Right now it's 5% - maybe
put it to 6%?

[ssid, 2017-05-03 18:48:08]

Patchset #2 (id:20001) has been deleted

[dmurph, 2017-05-03 21:58:19]

https://codereview.chromium.org/2855943003/diff/40001/storage/browser/blob/bl...
File storage/browser/blob/blob_memory_controller.cc (right):

https://codereview.chromium.org/2855943003/diff/40001/storage/browser/blob/bl...
storage/browser/blob/blob_memory_controller.cc:87: static_cast<uint64_t>(6 *
disk_size / 1000ll);
How about doing "3ll * disk_size  / 50" (6%, tries to prevent overflow, and
corrects from .6%)

[ssid, 2017-05-03 22:02:44]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-03 22:03:44]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dmurph, 2017-05-03 23:51:57]

https://codereview.chromium.org/2855943003/diff/60001/storage/common/blob_sto...
File storage/common/blob_storage/blob_storage_constants.h (right):

https://codereview.chromium.org/2855943003/diff/60001/storage/common/blob_sto...
storage/common/blob_storage/blob_storage_constants.h:25: constexpr uint64_t
kDefaultMinPageFileSize = 1024u * 1024;
Instead of putting this here, can you put this customization in the
CalculateBlobStorageLimitsImpl method?

[dmurph, 2017-05-03 23:53:14]

https://codereview.chromium.org/2855943003/diff/60001/storage/common/blob_sto...
File storage/common/blob_storage/blob_storage_constants.h (right):

https://codereview.chromium.org/2855943003/diff/60001/storage/common/blob_sto...
storage/common/blob_storage/blob_storage_constants.h:25: constexpr uint64_t
kDefaultMinPageFileSize = 1024u * 1024;
On 2017/05/03 23:51:57, dmurph wrote:
> Instead of putting this here, can you put this customization in the
> CalculateBlobStorageLimitsImpl method?

Actually - nvm I think this is fine. That method is purely customization based
on changing limits.

[dmurph, 2017-05-03 23:54:49]

lgtm

[ssid, 2017-05-04 00:30:00]

On 2017/05/03 23:54:49, dmurph wrote:
> lgtm

Since i changed the value of min_page_file_size,
CacheStorageBlobToDiskCacheTest.StreamLarge test started crashing.
The test adds a blob of 2MB. Previously it used to be in memory and now it gets
written to disk I guess. Attached the stack trace of the crash below. I am not
able to figure out why this crash happens. Do you have any ideas?

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0

Stack Trace:
05-03 23:28:02.599 26850 26850 F DEBUG   : backtrace:
05-03 23:28:02.599 26850 26850 F DEBUG   :     #00 pc 000ce5fc 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base8internal11TaskTracker14BeforePostTaskENS_20TaskShutdownBehaviorE+7)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #01 pc 000ce653 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base8internal11TaskTracker12WillPostTaskEPKNS0_4TaskE+10)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #02 pc 000cc4cb 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base8internal23SchedulerWorkerPoolImpl20PostTaskWithSequenceENSt6__ndk110unique_ptrINS0_4TaskENS2_14default_deleteIS4_EEEE13scoped_refptrINS0_8SequenceEE+26)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #03 pc 000cc7e7 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
05-03 23:28:02.599 26850 26850 F DEBUG   :     #04 pc 000c9e99 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base10TaskRunner8PostTaskERKN15tracked_objects8LocationENS_8CallbackIFvvELNS_8internal8CopyModeE0ELNS7_10RepeatModeE0EEE+36)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #05 pc 0004e47d 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
(_ZN7storage10ScopedFile5ResetEv+332)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #06 pc 0004e4e9 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
(_ZN7storage10ScopedFileD2Ev+4)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #07 pc 0004ebff 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
(_ZN7storage22ShareableFileReferenceD2Ev+134)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #08 pc 0004ec29 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
(_ZN7storage22ShareableFileReferenceD0Ev+4)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #09 pc 00043537 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
05-03 23:28:02.599 26850 26850 F DEBUG   :     #10 pc 00046457 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
05-03 23:28:02.599 26850 26850 F DEBUG   :     #11 pc 00043d0b 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
05-03 23:28:02.599 26850 26850 F DEBUG   :     #12 pc 000422c1 
/data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
05-03 23:28:02.599 26850 26850 F DEBUG   :     #13 pc 000d3e59 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
05-03 23:28:02.599 26850 26850 F DEBUG   :     #14 pc 0008d9d5 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base5debug13TaskAnnotator7RunTaskEPKcPNS_11PendingTaskE+160)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #15 pc 000a4ebd 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base11MessageLoop7RunTaskEPNS_11PendingTaskE+192)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #16 pc 000a5465 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base11MessageLoop21DeferOrRunPendingTaskENS_11PendingTaskE+96)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #17 pc 000a5689 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base11MessageLoop6DoWorkEv+184)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #18 pc 000a6551 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base19MessagePumpLibevent3RunEPNS_11MessagePump8DelegateE+112)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #19 pc 000a4acb 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base11MessageLoop10RunHandlerEv+66)
05-03 23:28:02.599 26850 26850 F DEBUG   :     #20 pc 000bad4b 
/data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
(_ZN4base7RunLoop3RunEv+30)
05-03 23:28:02.600 26850 26850 F DEBUG   :     #21 pc 008c282d 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #22 pc 002e70fb 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #23 pc 002e7145 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #24 pc 002e7155 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #25 pc 0091dbf9 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #26 pc 0091dca7 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #27 pc 0091f4dd 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #28 pc 0091aa4d 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #29 pc 008d924f 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #30 pc 008dcb9b 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #31 pc 00217957 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
05-03 23:28:02.600 26850 26850 F DEBUG   :     #32 pc 0089c629 
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so

[dmurph, 2017-05-04 00:44:02]

On 2017/05/04 00:30:00, ssid wrote:
> On 2017/05/03 23:54:49, dmurph wrote:
> > lgtm
> 
> Since i changed the value of min_page_file_size,
> CacheStorageBlobToDiskCacheTest.StreamLarge test started crashing.
> The test adds a blob of 2MB. Previously it used to be in memory and now it
gets
> written to disk I guess. Attached the stack trace of the crash below. I am not
> able to figure out why this crash happens. Do you have any ideas?
> 
> signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x0
> 
> Stack Trace:
> 05-03 23:28:02.599 26850 26850 F DEBUG   : backtrace:
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #00 pc 000ce5fc 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base8internal11TaskTracker14BeforePostTaskENS_20TaskShutdownBehaviorE+7)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #01 pc 000ce653 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base8internal11TaskTracker12WillPostTaskEPKNS0_4TaskE+10)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #02 pc 000cc4cb 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
>
(_ZN4base8internal23SchedulerWorkerPoolImpl20PostTaskWithSequenceENSt6__ndk110unique_ptrINS0_4TaskENS2_14default_deleteIS4_EEEE13scoped_refptrINS0_8SequenceEE+26)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #03 pc 000cc7e7 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #04 pc 000c9e99 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
>
(_ZN4base10TaskRunner8PostTaskERKN15tracked_objects8LocationENS_8CallbackIFvvELNS_8internal8CopyModeE0ELNS7_10RepeatModeE0EEE+36)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #05 pc 0004e47d 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> (_ZN7storage10ScopedFile5ResetEv+332)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #06 pc 0004e4e9 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> (_ZN7storage10ScopedFileD2Ev+4)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #07 pc 0004ebff 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> (_ZN7storage22ShareableFileReferenceD2Ev+134)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #08 pc 0004ec29 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> (_ZN7storage22ShareableFileReferenceD0Ev+4)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #09 pc 00043537 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #10 pc 00046457 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #11 pc 00043d0b 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #12 pc 000422c1 
> /data/app/org.chromium.native_test-2/lib/arm/libstorage_browser.cr.so
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #13 pc 000d3e59 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #14 pc 0008d9d5 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base5debug13TaskAnnotator7RunTaskEPKcPNS_11PendingTaskE+160)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #15 pc 000a4ebd 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base11MessageLoop7RunTaskEPNS_11PendingTaskE+192)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #16 pc 000a5465 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base11MessageLoop21DeferOrRunPendingTaskENS_11PendingTaskE+96)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #17 pc 000a5689 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base11MessageLoop6DoWorkEv+184)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #18 pc 000a6551 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base19MessagePumpLibevent3RunEPNS_11MessagePump8DelegateE+112)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #19 pc 000a4acb 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base11MessageLoop10RunHandlerEv+66)
> 05-03 23:28:02.599 26850 26850 F DEBUG   :     #20 pc 000bad4b 
> /data/app/org.chromium.native_test-2/lib/arm/libbase.cr.so
> (_ZN4base7RunLoop3RunEv+30)
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #21 pc 008c282d 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #22 pc 002e70fb 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #23 pc 002e7145 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #24 pc 002e7155 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #25 pc 0091dbf9 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #26 pc 0091dca7 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #27 pc 0091f4dd 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #28 pc 0091aa4d 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #29 pc 008d924f 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #30 pc 008dcb9b 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #31 pc 00217957 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so
> 05-03 23:28:02.600 26850 26850 F DEBUG   :     #32 pc 0089c629 
>
/data/app/org.chromium.native_test-2/lib/arm/lib_content_unittests__library.cr.so

It looks like the blob might be broken, check here:
https://cs.chromium.org/chromium/src/content/browser/cache_storage/cache_stor...
What is the status?

It shouldn't be trying to create files... as that's managed by the transport
system and not the backend. We'll definitely be 'overflowing' the backend, so
maybe it's trying to evict previous blobs to files, and then it's somehow caught
in a weird spot where not all the task runners were run?

It's a scoped file that's trying to be destroyed, so it looks like it's trying
to notify the listeners? Maybe that's it - tests might destroy things correctly?
We add the listener here:
https://cs.chromium.org/chromium/src/storage/browser/blob/blob_memory_control...

but that uses a weak pointer - any chance we can symbolize the stack?

...might be easier to make the memory > 2MB (so 2%)

[ssid, 2017-05-04 00:46:53]

Some thoughts:

scoped_async_task_scheduler_ is reset here:
https://cs.chromium.org/chromium/src/content/public/test/test_browser_thread_...

The runloop here tries to run a callback: base::Callback<void
(std::__ndk1::pair<storage::BlobMemoryController::FileCreationInfo, long long>)
https://cs.chromium.org/chromium/src/content/public/test/test_browser_thread_...

This causes ShareableFileReference::~ShareableFileReference() - >
storage::ScopedFile::Reset() which tries to post task on the already deleted
task runner and crashes.

[ssid, 2017-05-04 00:49:51]

Sorry I was not able to post the symbolized stack trace because of length of
msg.

The crashed stack trace here:
https://paste.googleplex.com/6193951333679104

The stack trace that destroys the TaskTracker is here:
  RELADDR   FUNCTION                                                            
                                                                                
                                          FILE:LINE
  0008d399  base::debug::StackTrace::StackTrace()                               
                                                                                
                                         
/s/chromium1/src/base/debug/stack_trace.cc:199
  0009f9ad  logging::LogMessage::~LogMessage()                                  
                                                                                
                                          /s/chromium1/src/base/logging.cc:552
  000cea7f  base::internal::TaskTracker::~TaskTracker()                         
                                                                                
                                         
/s/chromium1/src/base/task_scheduler/task_tracker.cc:205
  000cf419  base::internal::TaskTrackerPosix::~TaskTrackerPosix()               
                                                                                
                                         
/s/chromium1/src/base/task_scheduler/task_tracker_posix.h:26
  000ce549  base::internal::TaskSchedulerImpl::~TaskSchedulerImpl()             
                                                                                
                                         
/s/chromium1/src/base/task_scheduler/task_scheduler_impl.cc:82
  000ce56f  base::internal::TaskSchedulerImpl::~TaskSchedulerImpl()             
                                                                                
                                         
/s/chromium1/src/base/task_scheduler/task_scheduler_impl.cc:86
  000cdd51 
base::TaskScheduler::SetInstance(std::__ndk1::unique_ptr<base::TaskScheduler,
std::__ndk1::default_delete<base::TaskScheduler> >)                             
                                
/s/chromium1/src/base/task_scheduler/task_scheduler.cc:83
  008d62cd  base::test::ScopedAsyncTaskScheduler::~ScopedAsyncTaskScheduler()   
                                                                                
                                         
/s/chromium1/src/base/test/scoped_async_task_scheduler.cc:37
  v------> 
std::__ndk1::default_delete<base::test::ScopedAsyncTaskScheduler>::operator()(base::test::ScopedAsyncTaskScheduler*)
const                                                                     
/s/chromium1/src/third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2431
  v------>  std::__ndk1::unique_ptr<base::test::ScopedAsyncTaskScheduler,
std::__ndk1::default_delete<base::test::ScopedAsyncTaskScheduler>
>::reset(base::test::ScopedAsyncTaskScheduler*)                
/s/chromium1/src/third_party/android_tools/ndk/sources/cxx-stl/llvm-libc++/libcxx/include/memory:2630
  008c281d  content::TestBrowserThreadBundle::~TestBrowserThreadBundle()        
                                                                                
                                         
/s/chromium1/src/content/public/test/test_browser_thread_bundle.cc:59
  002e70fd  ~CacheStorageBlobToDiskCacheTest                                    
                                                                                
                                         
/s/chromium1/src/content/browser/cache_storage/cache_storage_blob_to_disk_cache_unittest.cc:93
  002e7147  ~CacheStorageBlobToDiskCacheTest_StreamLarge_Test                   
                                                                                
                                         
/s/chromium1/src/content/browser/cache_storage/cache_storage_blob_to_disk_cache_unittest.cc:214
  002e7157  ~CacheStorageBlobToDiskCacheTest_StreamLarge_Test                   
                                                                                
                                         
/s/chromium1/src/content/browser/cache_storage/cache_storage_blob_to_disk_cache_unittest.cc:214
  0091dbfb  testing::TestInfo::Run()                                            
                                                                                
                                         
/s/chromium1/src/testing/gtest/src/gtest.cc:2662
  0091dca9  testing::TestCase::Run()                                            
                                                                                
                                         
/s/chromium1/src/testing/gtest/src/gtest.cc:2774
  0091f4df  testing::internal::UnitTestImpl::RunAllTests()                      
                                                                                
                                         
/s/chromium1/src/testing/gtest/src/gtest.cc:4647
  v------>  bool
testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,
bool>(testing::internal::UnitTestImpl*, bool
(testing::internal::UnitTestImpl::*)(), char const*) 
/s/chromium1/src/testing/gtest/src/gtest.cc:2458
  0091aa4f  testing::UnitTest::Run()                                            
                                                                                
                                         
/s/chromium1/src/testing/gtest/src/gtest.cc:4258
  v------>  RUN_ALL_TESTS()                                                     
                                                                                
                                         
/s/chromium1/src/testing/gtest/include/gtest/gtest.h:22

[ssid, 2017-05-04 00:51:37]

> ...might be easier to make the memory > 2MB (so 2%)

True. But I was just scared I might be introducing some bug in the code causing
crash in production. If you think this can only happen in tests, I can change
the limit to 2MB.

[dmurph, 2017-05-04 01:06:39]

On 2017/05/04 00:51:37, ssid wrote:
> > ...might be easier to make the memory > 2MB (so 2%)
> 
> True. But I was just scared I might be introducing some bug in the code
causing
> crash in production. If you think this can only happen in tests, I can change
> the limit to 2MB.

Ah, yeah so this sounds like it's because the task that is supposed to delete
the files can't be run because that task runner is dead. Huh. Not sure how to
change the bundle to support that... and that task runner is supposed to ignore
on shutdown. I would say changing the limit a little is the easiest fix (you'll
probably have to do a little over 2mb - I'm guessing that there's another blob
or something that's being evicted for the new item). Ah, yeah, the test blob for
that test case.

Otherwise you might have to figure out how to keep that task runner alive? I'm
not super familiar with how the thread bundle works - that task runner is passed
into the memory controller is initialized here:
https://cs.chromium.org/chromium/src/content/browser/blob_storage/chrome_blob...

[ssid, 2017-05-04 01:13:16]

On 2017/05/04 01:06:39, dmurph wrote:
> On 2017/05/04 00:51:37, ssid wrote:
> > > ...might be easier to make the memory > 2MB (so 2%)
> > 
> > True. But I was just scared I might be introducing some bug in the code
> causing
> > crash in production. If you think this can only happen in tests, I can
change
> > the limit to 2MB.
> 
> Ah, yeah so this sounds like it's because the task that is supposed to delete
> the files can't be run because that task runner is dead. Huh. Not sure how to
> change the bundle to support that... and that task runner is supposed to
ignore
> on shutdown. I would say changing the limit a little is the easiest fix
(you'll
> probably have to do a little over 2mb - I'm guessing that there's another blob
> or something that's being evicted for the new item). Ah, yeah, the test blob
for
> that test case.
> 
> Otherwise you might have to figure out how to keep that task runner alive? I'm
> not super familiar with how the thread bundle works - that task runner is
passed
> into the memory controller is initialized here:
>
https://cs.chromium.org/chromium/src/content/browser/blob_storage/chrome_blob...


um Yes.
The CreateFileAndWriteItems() is run after the file taskrunner is destroyed.
When the FileCreationInfo is destroyed at the end of this function, the
|file_reference| loses all the references and is destructed. This cases
ScopedFile::Reset() to post task on file task runner.
I guess it wouldn't happen in normal case. So, making the limit 2.5MB to make
bots happy.

[ssid, 2017-05-04 01:13:26]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 01:13:41]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ssid, 2017-05-04 01:16:06]

Thanks for your help!

[commit-bot: I haz the power, 2017-05-04 03:08:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 03:08:23]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[dmurph, 2017-05-04 21:24:54]

On 2017/05/04 01:16:06, ssid wrote:
> Thanks for your help!

Also, about 4 months ago this limit was stuck to 500MB for in-memory - so the
fact you can change this per platform and stuff is very new and AWESOME <insert
awesome gif>

[ssid, 2017-05-04 21:39:16]

More browsertest failures and I fixed the test by lowering the size of blobs
that indexeddb writes.
Does that seem fine?

[ssid, 2017-05-04 21:40:24]

Patchset #5 (id:100001) has been deleted

[ssid, 2017-05-04 22:00:41]

The CQ bit was checked by ssid@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-04 22:01:06]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[ssid, 2017-05-04 22:01:25]

ssid@chromium.org changed reviewers:
+ jsbell@chromium.org

[ssid, 2017-05-04 22:01:26]

+jsbell for the indexed_db test fixes.

[jsbell, 2017-05-04 22:11:37]

lgtm with a possible tweak, but fine as is

https://codereview.chromium.org/2855943003/diff/120001/content/browser/indexe...
File content/browser/indexed_db/indexed_db_browsertest.cc (right):

https://codereview.chromium.org/2855943003/diff/120001/content/browser/indexe...
content/browser/indexed_db/indexed_db_browsertest.cc:492:
SimpleTest(GetTestUrl("indexeddb", "write_4mb_blob.html"));
If you wanted to get fancy we could make this parameterized:
write_blob_mbs.html#4

... and then in the script use Number(document.location.hash.substring(1)) *
1024 * 1024

[commit-bot: I haz the power, 2017-05-04 23:07:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-04 23:07:10]

Dry run: This issue passed the CQ dry run.

[ssid, 2017-05-05 00:34:24]

https://codereview.chromium.org/2855943003/diff/120001/content/browser/indexe...
File content/browser/indexed_db/indexed_db_browsertest.cc (right):

https://codereview.chromium.org/2855943003/diff/120001/content/browser/indexe...
content/browser/indexed_db/indexed_db_browsertest.cc:492:
SimpleTest(GetTestUrl("indexeddb", "write_4mb_blob.html"));
On 2017/05/04 22:11:37, jsbell wrote:
> If you wanted to get fancy we could make this parameterized:
> write_blob_mbs.html#4
> 
> ... and then in the script use Number(document.location.hash.substring(1)) *
> 1024 * 1024

hm actually changing the file name causes the test unable to read the file.
(content::GetTestFilePath does not split the name).

[ssid, 2017-05-05 00:34:29]

The CQ bit was checked by ssid@chromium.org

[ssid, 2017-05-05 00:34:29]

The patchset sent to the CQ was uploaded after l-g-t-m from dmurph@chromium.org
Link to the patchset: https://codereview.chromium.org/2855943003/#ps120001
(title: "Fix  IndexedDBBrowserTest.")

[commit-bot: I haz the power, 2017-05-05 00:35:01]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-05 00:41:51]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1493944469329410,
"parent_rev": "6d0b3151c08b8056637938953c936719e583dc54", "commit_rev":
"81a539c78aaf463a2aeb90c10366e586f6d85ab5"}

[commit-bot: I haz the power, 2017-05-05 00:42:27]

Description was changed from

==========
Reduce the in-memory blob storgae limit on Android

BUG=715859
==========

to

==========
Reduce the in-memory blob storgae limit on Android

BUG=715859

Review-Url: https://codereview.chromium.org/2855943003
Cr-Commit-Position: refs/heads/master@{#469550}
Committed:
https://chromium.googlesource.com/chromium/src/+/81a539c78aaf463a2aeb90c10366...
==========

[commit-bot: I haz the power, 2017-05-05 00:42:28]

Committed patchset #5 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/81a539c78aaf463a2aeb90c10366...

[jsbell, 2017-05-05 18:53:05]

https://codereview.chromium.org/2855943003/diff/120001/content/browser/indexe...
File content/browser/indexed_db/indexed_db_browsertest.cc (right):

https://codereview.chromium.org/2855943003/diff/120001/content/browser/indexe...
content/browser/indexed_db/indexed_db_browsertest.cc:492:
SimpleTest(GetTestUrl("indexeddb", "write_4mb_blob.html"));
On 2017/05/05 00:34:23, ssid wrote:
> On 2017/05/04 22:11:37, jsbell wrote:
> > If you wanted to get fancy we could make this parameterized:
> > write_blob_mbs.html#4
> > 
> > ... and then in the script use Number(document.location.hash.substring(1)) *
> > 1024 * 1024
> 
> hm actually changing the file name causes the test unable to read the file.
> (content::GetTestFilePath does not split the name).

huh. It looked like we used that pattern elsewhere but maybe it requires more
refactoring.

Anyway, glad this is in, we can tweak it later.

\o/