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

Issue 2199393002: Android: Make the spare renderer accessible to all Chrome tabs. (Closed)

Created:
4 years, 4 months ago by Benoit L
Modified:
4 years, 4 months ago
Reviewers:
Yusuf, Maria
CC:
chromium-reviews, lizeb+watch-custom-tabs_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Android: Make the spare renderer accessible to all Chrome tabs. In Custom Tabs, an external application can "warm up" Chrome. This creates and initializes a spare renderer, and has been found to be a significant optimization for loading performance. This patch moves the spare renderer logic out of Custom Tabs, and makes it accessible and useful for regular navigations in Chrome. Note that this patch doesn't add new call sites for the spare renderer creation. BUG=633964 Committed: https://crrev.com/f7ce563033df1df90b314e4f727596c93c2b3d6d Cr-Commit-Position: refs/heads/master@{#412510}

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : Moar tests. #

Total comments: 8

Patch Set 4 : Address comments. #

Patch Set 5 : Make FindBugs happy. #

Patch Set 6 : Fix a test issue exposed by this CL. #

Total comments: 6

Patch Set 7 : Address comments. #

Messages

Total messages: 39 (26 generated)
Benoit L
4 years, 4 months ago (2016-08-03 14:55:29 UTC) #7
Yusuf
lgtm with a few nits and a suggestion. https://codereview.chromium.org/2199393002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java File chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java (right): https://codereview.chromium.org/2199393002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java#newcode229 chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java:229: public ...
4 years, 4 months ago (2016-08-05 05:32:43 UTC) #8
Benoit L
Thanks! Adding mariakhomenko@ as a reviewer, PTAL. https://codereview.chromium.org/2199393002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java File chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java (right): https://codereview.chromium.org/2199393002/diff/40001/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java#newcode229 chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java:229: public WebContents ...
4 years, 4 months ago (2016-08-08 09:48:20 UTC) #10
Maria
On 2016/08/08 09:48:20, Benoit L wrote: > Thanks! > > Adding mariakhomenko@ as a reviewer, ...
4 years, 4 months ago (2016-08-10 23:54:52 UTC) #23
Maria
lgtm Thanks for adding all the tests! Looks good, I just have a few nits. ...
4 years, 4 months ago (2016-08-11 17:34:37 UTC) #24
Benoit L
Done, thanks! https://codereview.chromium.org/2199393002/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java File chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java (right): https://codereview.chromium.org/2199393002/diff/100001/chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java#newcode240 chrome/android/java/src/org/chromium/chrome/browser/WarmupManager.java:240: @VisibleForTesting On 2016/08/11 17:34:37, Maria wrote: > ...
4 years, 4 months ago (2016-08-17 11:31:33 UTC) #25
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/2199393002/120001
4 years, 4 months ago (2016-08-17 13:24:45 UTC) #32
commit-bot: I haz the power
Committed patchset #7 (id:120001)
4 years, 4 months ago (2016-08-17 13:27:45 UTC) #33
commit-bot: I haz the power
Patchset 7 (id:??) landed as https://crrev.com/f7ce563033df1df90b314e4f727596c93c2b3d6d Cr-Commit-Position: refs/heads/master@{#412510}
4 years, 4 months ago (2016-08-17 13:29:30 UTC) #35
boliu
WarmupManagerTest#testCreateAndTakeSpareRenderer failed on 3 different bots, I'm reverting
4 years, 4 months ago (2016-08-17 16:59:26 UTC) #36
boliu
On 2016/08/17 16:59:26, boliu wrote: > WarmupManagerTest#testCreateAndTakeSpareRenderer failed on 3 different bots, I'm > reverting ...
4 years, 4 months ago (2016-08-17 17:01:24 UTC) #37
boliu
A revert of this CL (patchset #7 id:120001) has been created in https://codereview.chromium.org/2254863002/ by boliu@chromium.org. ...
4 years, 4 months ago (2016-08-17 17:02:19 UTC) #38
boliu
4 years, 4 months ago (2016-08-17 17:13:46 UTC) #39
Message was sent while issue was closed.
it's a java exception turned crash, native part of the stack looks semi-bogus
though..

0210f:  08-17 15:38:34.579 31317 31317 W System.err: java.lang.AssertionError
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
org.chromium.content.browser.webcontents.WebContentsObserverProxy.destroy(WebContentsObserverProxy.java:257)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
org.chromium.content.browser.webcontents.WebContentsImpl.nativeDestroyWebContents(Native
Method)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
org.chromium.content.browser.webcontents.WebContentsImpl.destroy(WebContentsImpl.java:142)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
org.chromium.chrome.browser.WarmupManagerTest$6.run(WarmupManagerTest.java:94)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
android.os.Handler.handleCallback(Handler.java:733)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
android.os.Handler.dispatchMessage(Handler.java:95)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
android.os.Looper.loop(Looper.java:136)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
android.app.ActivityThread.main(ActivityThread.java:5001)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
java.lang.reflect.Method.invokeNative(Native Method)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
java.lang.reflect.Method.invoke(Method.java:515)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:785)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
com.android.internal.os.ZygoteInit.main(ZygoteInit.java:601)
0210f:  08-17 15:38:34.579 31317 31317 W System.err:  at
dalvik.system.NativeStart.main(Native Method)
0210f:  08-17 15:38:34.599 31317 31337 D dalvikvm: GC_EXPLICIT freed 122K, 5%
free 17065K/17932K, paused 2ms+2ms, total 19ms
0210f:  08-17 15:38:34.609 31317 31317 F chromium: [FATAL:jni_android.cc(236)]
Please include Java exception stack in crash report

  00cc9e39 
content::DevToolsProtocolDispatcher::OnBrowserCreateTarget(content::DevToolsCommandId,
std::__1::unique_ptr<base::DictionaryValue,
std::__1::default_delete<base::DictionaryValue> >)+156                          
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                         
/b/c/b/android/src/out/Debug/gen/content/browser/devtools/protocol/devtools_protocol_dispatcher.cc:1720
  v------>  base::internal::BindState<void
(media::MediaDecoderJob::*)(media::MediaCodecStatus, bool, base::TimeDelta,
base::TimeDelta), base::internal::UnretainedWrapper<media::MediaDecoderJob>,
media::MediaCodecStatus, bool, base::TimeDelta, base::TimeDelta>::BindState<void
(media::MediaDecoderJob::*)(media::MediaCodecStatus, bool, base::TimeDelta,
base::TimeDelta), base::internal::UnretainedWrapper<media::MediaDecoderJob>,
media::MediaCodecStatus, bool, base::TimeDelta const&, base::TimeDelta
const&>(void (media::MediaDecoderJob::*&&)(media::MediaCodecStatus, bool,
base::TimeDelta, base::TimeDelta),
base::internal::UnretainedWrapper<media::MediaDecoderJob>&&,
media::MediaCodecStatus&&, bool&&, base::TimeDelta const&, base::TimeDelta
const&)                                                                         
      /b/c/b/android/src/base/bind_internal.h:384
  v------>  base::Callback<base::internal::MakeUnboundRunTypeImpl<void
(media::MediaDecoderJob::*)(media::MediaCodecStatus, bool, base::TimeDelta,
base::TimeDelta), base::internal::UnretainedWrapper<media::MediaDecoderJob>,
media::MediaCodecStatus, bool, base::TimeDelta const&, base::TimeDelta
const&>::Type, (base::internal::CopyMode)1> base::Bind<void
(media::MediaDecoderJob::*)(media::MediaCodecStatus, bool, base::TimeDelta,
base::TimeDelta), base::internal::UnretainedWrapper<media::MediaDecoderJob>,
media::MediaCodecStatus, bool, base::TimeDelta const&, base::TimeDelta
const&>(void (media::MediaDecoderJob::*&&)(media::MediaCodecStatus, bool,
base::TimeDelta, base::TimeDelta),
base::internal::UnretainedWrapper<media::MediaDecoderJob>&&,
media::MediaCodecStatus&&, bool&&, base::TimeDelta const&, base::TimeDelta
const&)  /b/c/b/android/src/base/bind.h:38
  00c9a821  media::MediaDecoderJob::DecodeCurrentAccessUnit(base::TimeTicks,
base::TimeDelta)+284                                                            
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                          
/b/c/b/android/src/media/base/android/media_decoder_job.cc:358
  00c9aaf1  media::MediaDecoderJob::DecodeCurrentAccessUnit(base::TimeTicks,
base::TimeDelta)+1004                                                           
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                          
/b/c/b/android/src/media/base/android/media_decoder_job.cc:375
  v------>  base::Callback<void (net::NetworkChangeNotifier::ConnectionType),
(base::internal::CopyMode)1>::Callback(base::Callback<void
(net::NetworkChangeNotifier::ConnectionType), (base::internal::CopyMode)1>
const&)                                                                         
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                                                
                                                     
/b/c/b/android/src/base/callback.h:350
  v------>  base::internal::BindState<base::Callback<void
(net::NetworkChangeNotifier::ConnectionType), (base::internal::CopyMode)1>,
net::NetworkChangeNotifier::ConnectionType>::BindState<base::Callback<void
(net::NetworkChangeNotifier::ConnectionType), (base::internal::CopyMode)1>&,
net::NetworkChangeNotifier::ConnectionType>(base::Callback<void
(net::NetworkChangeNotifier::ConnectionType), (base::internal::CopyMode)1>&,
net::NetworkChangeNotifier::ConnectionType&&)                                   
                                                                                
                                                                                
                                                                                
                                                                                
 /b/c/b/android/src/base/bind_internal.h:384
  v------> 
base::Callback<base::internal::MakeUnboundRunTypeImpl<base::Callback<void
(net::NetworkChangeNotifier::ConnectionType), (base::internal::CopyMode)1>&,
net::NetworkChangeNotifier::ConnectionType>::Type, (base::internal::CopyMode)1>
base::Bind<base::Callback<void (net::NetworkChangeNotifier::ConnectionType),
(base::internal::CopyMode)1>&,
net::NetworkChangeNotifier::ConnectionType>(base::Callback<void
(net::NetworkChangeNotifier::ConnectionType), (base::internal::CopyMode)1>&,
net::NetworkChangeNotifier::ConnectionType&&)                                   
                                                                                
                                                                                
                                                                                
                 /b/c/b/android/src/base/bind.h:38
  00cd6529 
content::BackgroundSyncNetworkObserverAndroid::Observer::NotifyConnectionTypeChanged(_JNIEnv*,
base::android::JavaParamRef<_jobject*> const&, int)+140

Powered by Google App Engine
This is Rietveld 408576698