Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1569643002:
Reland Pull the Activity context from WindowAndroid if possible (Closed)
|
Created:
4 years, 11 months ago by Yusuf Modified:
4 years, 11 months ago CC:
chromium-reviews, darin-cc_chromium.org, jam, android-webview-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReland Pull the Activity context from WindowAndroid if possible
WebView uses ContentViewCore's context for displaying dialogs and
wraps it at construction time. This change makes sure that if there
is an activity that can be reached through WindowAndroid, we use
that as the context inside ColorChooser. Also make sure if there is no activity, we
dont attempt to create a color chooser and try to show it.
Update ActivityWindowAndroid to take a Context type so
that in Android WebView, the activity context can remain wrapped.
BUG=550410, 570429
Committed: https://crrev.com/235c2e05a35759c50b0f106ab1f7aec3142eebb1
Cr-Commit-Position: refs/heads/master@{#368702}
Patch Set 1 #Patch Set 2 : Added landmine #
Total comments: 1
Patch Set 3 : Fix ChromeWindow constructor #Patch Set 4 : Revert the changes in ChromeWindow as they are not needed anymore #
Messages
Total messages: 30 (8 generated)
yusufo@chromium.org changed reviewers: + boliu@chromium.org, jbudorick@chromium.org, newt@chromium.org
PTAL, I will try to reland this with a landmine as a last resort over the weekend. This previous failed compile on perf bots for reasons I was not able to understand. Verified all Android related targets build fine on gn and gyp. build/ jbudorick@ webview/ boliu@ ui/ newt@ will TBR tedchoc if for content/public if everyone else is OK with this relanding.
Reverted CL here https://codereview.chromium.org/1528733002
https://codereview.chromium.org/1569643002/diff/20001/build/get_landmines.py File build/get_landmines.py (right): https://codereview.chromium.org/1569643002/diff/20001/build/get_landmines.py#... build/get_landmines.py:27: # DO NOT add landmines as part of a regular CL. Landmines are a last-effort If it's at all possible, let's try to avoid a landmine.
Are there any changes from the previous CL besides the addition of the landmine? Also, could you link to the previous CL from here?
On 2016/01/08 23:54:50, newt wrote: > Are there any changes from the previous CL besides the addition of the landmine? No. > > Also, could you link to the previous CL from here? Linked in comments.
On 2016/01/08 23:54:35, jbudorick wrote: > https://codereview.chromium.org/1569643002/diff/20001/build/get_landmines.py > File build/get_landmines.py (right): > > https://codereview.chromium.org/1569643002/diff/20001/build/get_landmines.py#... > build/get_landmines.py:27: # DO NOT add landmines as part of a regular CL. > Landmines are a last-effort > If it's at all possible, let's try to avoid a landmine. Frankly I am out of ideas on this one. Tried on the day of the revert as well, but there is simply no reason me, boliu and that day's clank sheriff could find for the compile failure. All related targets are already included and should be building fine. And now the build history for the failures is gone as well.
On 2016/01/08 23:59:19, Yusuf wrote: > On 2016/01/08 23:54:35, jbudorick wrote: > > https://codereview.chromium.org/1569643002/diff/20001/build/get_landmines.py > > File build/get_landmines.py (right): > > > > > https://codereview.chromium.org/1569643002/diff/20001/build/get_landmines.py#... > > build/get_landmines.py:27: # DO NOT add landmines as part of a regular CL. > > Landmines are a last-effort > > If it's at all possible, let's try to avoid a landmine. > > Frankly I am out of ideas on this one. Tried on the day of the revert as well, > but there is simply no reason me, boliu and that day's clank sheriff could find > for the compile failure. All related targets are already included and should be > building fine. And now the build history for the failures is gone as well. fwiw, the error was some chrome java target (which I don't remember now..) still trying to find "public ActivityWindowAndroid(Activity)" constructor in the ui jar, even though it's removed (parameter changed to Context) in this CL. Afaict everything was rebuilt correctly. And none of us could reproduce the error locally. At the time, the gyp dependency looked correct to me, and I grepped through the log to see ui.jar recompiled. CQ bots were fine, so could have been a target built on some other bots. None of us could reproduce the error locally. Honestly landmine is a shot in the dark too, since I don't understand the cause :/
lgtm for my parts while we debate the landmine thing..
yusufo@chromium.org changed reviewers: + tedchoc@chromium.org
Thanks to jbudorick's help, I found the real culprit. No need for landmines now. Actually it wouldn't have helped, so thanks for not letting me land it :) Adding back tedchoc@ for a rubberstamp approval since he will be back on 1/11. Ted, this is the same CL except the change in ChromeWindow constructor.
On 2016/01/10 19:24:46, Yusuf wrote: > Thanks to jbudorick's help, I found the real culprit. No need for landmines now. > Actually it wouldn't have helped, so thanks for not letting me land it :) > > Adding back tedchoc@ for a rubberstamp approval since he will be back on 1/11. > > Ted, this is the same CL except the change in ChromeWindow constructor. Hmm, javadocs? So why didn't the failure reproduce loally?
On 2016/01/10 19:30:05, boliu wrote: > On 2016/01/10 19:24:46, Yusuf wrote: > > Thanks to jbudorick's help, I found the real culprit. No need for landmines > now. > > Actually it wouldn't have helped, so thanks for not letting me land it :) > > > > Adding back tedchoc@ for a rubberstamp approval since he will be back on 1/11. > > > > Ted, this is the same CL except the change in ChromeWindow constructor. > > Hmm, javadocs? So why didn't the failure reproduce loally? No, the real issue is the constructor for ChromeWindow. This class extends ActivityWindowAndroid and my CL was changing the constructor without modifying the one for ChromeWindow. The failure not reproducing locally is still a mystery. ChromeWindow is not used upstream which is probably related, but still chrome_apk should have caught this I think. At the end I reproduced the compile error after an official build with some other GYP_DEFINES
On 2016/01/10 19:50:36, Yusuf wrote: > On 2016/01/10 19:30:05, boliu wrote: > > On 2016/01/10 19:24:46, Yusuf wrote: > > > Thanks to jbudorick's help, I found the real culprit. No need for landmines > > now. > > > Actually it wouldn't have helped, so thanks for not letting me land it :) > > > > > > Adding back tedchoc@ for a rubberstamp approval since he will be back on > 1/11. > > > > > > Ted, this is the same CL except the change in ChromeWindow constructor. > > > > Hmm, javadocs? So why didn't the failure reproduce loally? > > No, the real issue is the constructor for ChromeWindow. This class extends > ActivityWindowAndroid and my CL was changing the constructor without modifying > the one for ChromeWindow. Actually I don't get that part. The old ChromeWindow code is legal java code afaict? > The failure not reproducing locally is still a > mystery. > ChromeWindow is not used upstream which is probably related, but still > chrome_apk > should have caught this I think. At the end I reproduced the compile error after > an official build with some other GYP_DEFINES
Talked to Yusuf offline, then tested more.. Any white-space change to
ChromeWindow.java would have fixed the the build error, although simply touching
the file isn't enough. By extension, SelectFileDialogTest.java probably also
needs such a change too. Fwiw land mine would have worked too.
Fwiw the javac.py command for chrome_java is issued, so the gyp dependencies are
correct. I *think* the actual javac command never happans, this is a bug in
javac.py. Haven't worked out exactly what yet.
Anyway, this is the eventual error with proguard.
"""
util.build_utils.CalledProcessError: Command failed: ( cd
/android/chromium/src/clank/native; java -jar
../../third_party/proguard/lib/proguard.jar -forceprocessing -libraryjars
../../third_party/android_platform/webview/frameworks_6.0.jar -injars
/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v7/mediarouter/libs/android-support-v7-mediarouter.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar:../../out/Release/lib.java/android_support_v7_appcompat_javalib.jar:../../out/Release/lib.java/android_support_v7_mediarouter_javalib.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v13/android-support-v13.jar:../../out/Release/lib.java/android_google_play_services_first_party_released_javalib_no_res.pre.jar:../../out/Release/lib.java/android_google_play_services_first_party_released_javalib.jar:../../out/Release/lib.java/android_gsf_client_javalib.pre.jar:../../out/Release/lib.java/protobuf_nano_javalib.jar:../../out/Release/lib.java/context_proto_java.jar:../../out/Release/lib.java/document_tab_model_info_proto_java.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/multidex/library/libs/android-support-multidex.jar:../../out/Release/lib.java/jsr_305_javalib.jar:../../out/Release/lib.java/base_java.jar:../../out/Release/lib.java/bookmarks_java.jar:../../out/Release/lib.java/mojo_public_java.jar:../../out/Release/lib.java/mojo_bindings_java.jar:../../out/Release/lib.java/device_battery_java.jar:../../out/Release/lib.java/device_bluetooth_java.jar:../../out/Release/lib.java/device_usb_java.jar:../../out/Release/lib.java/device_vibration_java.jar:../../out/Release/lib.java/media_java.jar:../../out/Release/lib.java/mojo_system_java.jar:../../out/Release/lib.java/net_java.jar:../../out/Release/lib.java/ui_java.jar:../../out/Release/lib.java/blink_headers_java.jar:../../out/Release/lib.java/ui_accessibility_java.jar:../../out/Release/lib.java/midi_java.jar:../../third_party/cardboard-java/src/CardboardSample/libs/cardboard.jar:../../out/Release/lib.java/content_java.jar:../../out/Release/lib.java/dom_distiller_java.jar:/android/chromium/src/third_party/android_tools/sdk//extras/google/gcm/gcm-client/dist/gcm.jar:../../out/Release/lib.java/cacheinvalidation_proto_java.jar:../../out/Release/lib.java/cacheinvalidation_javalib.jar:../../out/Release/lib.java/sync_java.jar:../../out/Release/lib.java/gcm_driver_java.jar:../../out/Release/lib.java/invalidation_proto_java.jar:../../out/Release/lib.java/invalidation_java.jar:../../out/Release/lib.java/navigation_interception_java.jar:../../out/Release/lib.java/precache_java.jar:../../out/Release/lib.java/safe_json_java.jar:../../out/Release/lib.java/service_tab_launcher_java.jar:../../out/Release/lib.java/signin_core_browser_java.jar:../../out/Release/lib.java/variations_java.jar:../../out/Release/lib.java/web_contents_delegate_android_java.jar:../../out/Release/lib.java/web_restriction_java.jar:../../out/Release/lib.java/printing_java.jar:../../out/Release/lib.java/android_data_chart_java.jar:../../out/Release/lib.java/android_media_java.jar:../../out/Release/lib.java/android_swipe_refresh_java.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v7/recyclerview/libs/android-support-v7-recyclerview.jar:../../out/Release/lib.java/android_support_v7_recyclerview_javalib.jar:../../out/Release/lib.java/gif_player_java.jar:../../out/Release/lib.java/policy_java.jar:../../out/Release/lib.java/chrome_java.jar:../../out/Release/lib.java/clank_java.jar:../../out/Release/lib.java/android_webview_google_java.jar:../../out/Release/lib.java/external_video_surface_java.jar:../../out/Release/lib.java/android_webview_java.jar:../../out/Release/lib.java/system_webview_glue_java.jar:../../out/Release/lib.java/chromium_apk_monochrome_apk.jar
-include ../../android_webview/apk/java/proguard.flags -include
../../chrome/android/java/proguard.flags -include
../../clank/java/proguard.flags -include
../../out/Release/monochrome_apk/proguard.txt -outjars
../../out/Release/monochrome_apk/obfuscated.jar -dump
../../out/Release/monochrome_apk/obfuscated.jar.dump -printseeds
../../out/Release/monochrome_apk/obfuscated.jar.seeds -printusage
../../out/Release/monochrome_apk/obfuscated.jar.usage -printmapping
../../out/Release/monochrome_apk/obfuscated.jar.mapping )
Warning: org.chromium.chrome.browser.ChromeApplication: can't find referenced
method 'ActivityWindowAndroid(android.app.Activity)' in class
org.chromium.ui.base.ActivityWindowAndroid
Warning: org.chromium.chrome.browser.ChromeWindow: can't find referenced method
'ActivityWindowAndroid(android.app.Activity)' in class
org.chromium.ui.base.ActivityWindowAndroid
Warning: there were 2 unresolved references to program class members.
Your input classes appear to be inconsistent.
You may need to recompile them and try again.
Alternatively, you may have to specify the option
'-dontskipnonpubliclibraryclassmembers'.
Error: Please correct the above warnings first.
"""
Description was changed from ========== Reland Pull the Activity context from WindowAndroid if possible WebView uses ContentViewCore's context for displaying dialogs and wraps it at construction time. This change makes sure that if there is an activity that can be reached through WindowAndroid, we use that as the context inside ColorChooser. Also make sure if there is no activity, we dont attempt to create a color chooser and try to show it. Update ActivityWindowAndroid to take a Context type so that in Android WebView, the activity context can remain wrapped. BUG=550410, 570429 ========== to ========== Reland Pull the Activity context from WindowAndroid if possible WebView uses ContentViewCore's context for displaying dialogs and wraps it at construction time. This change makes sure that if there is an activity that can be reached through WindowAndroid, we use that as the context inside ColorChooser. Also make sure if there is no activity, we dont attempt to create a color chooser and try to show it. Update ActivityWindowAndroid to take a Context type so that in Android WebView, the activity context can remain wrapped. BUG=550410, 570429 ==========
jbudorick@chromium.org changed reviewers: + agrieve@chromium.org
On 2016/01/10 22:21:31, boliu wrote: > Talked to Yusuf offline, then tested more.. Any white-space change to > ChromeWindow.java would have fixed the the build error, although simply touching > the file isn't enough. By extension, SelectFileDialogTest.java probably also > needs such a change too. Fwiw land mine would have worked too. > > Fwiw the javac.py command for chrome_java is issued, so the gyp dependencies are > correct. I *think* the actual javac command never happans, this is a bug in > javac.py. Haven't worked out exactly what yet. agrieve, any thoughts on what might have happened here? > > Anyway, this is the eventual error with proguard. > > """ > util.build_utils.CalledProcessError: Command failed: ( cd > /android/chromium/src/clank/native; java -jar > ../../third_party/proguard/lib/proguard.jar -forceprocessing -libraryjars > ../../third_party/android_platform/webview/frameworks_6.0.jar -injars > /android/chromium/src/third_party/android_tools/sdk//extras/android/support/v7/mediarouter/libs/android-support-v7-mediarouter.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v7/appcompat/libs/android-support-v7-appcompat.jar:../../out/Release/lib.java/android_support_v7_appcompat_javalib.jar:../../out/Release/lib.java/android_support_v7_mediarouter_javalib.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v13/android-support-v13.jar:../../out/Release/lib.java/android_google_play_services_first_party_released_javalib_no_res.pre.jar:../../out/Release/lib.java/android_google_play_services_first_party_released_javalib.jar:../../out/Release/lib.java/android_gsf_client_javalib.pre.jar:../../out/Release/lib.java/protobuf_nano_javalib.jar:../../out/Release/lib.java/context_proto_java.jar:../../out/Release/lib.java/document_tab_model_info_proto_java.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/multidex/library/libs/android-support-multidex.jar:../../out/Release/lib.java/jsr_305_javalib.jar:../../out/Release/lib.java/base_java.jar:../../out/Release/lib.java/bookmarks_java.jar:../../out/Release/lib.java/mojo_public_java.jar:../../out/Release/lib.java/mojo_bindings_java.jar:../../out/Release/lib.java/device_battery_java.jar:../../out/Release/lib.java/device_bluetooth_java.jar:../../out/Release/lib.java/device_usb_java.jar:../../out/Release/lib.java/device_vibration_java.jar:../../out/Release/lib.java/media_java.jar:../../out/Release/lib.java/mojo_system_java.jar:../../out/Release/lib.java/net_java.jar:../../out/Release/lib.java/ui_java.jar:../../out/Release/lib.java/blink_headers_java.jar:../../out/Release/lib.java/ui_accessibility_java.jar:../../out/Release/lib.java/midi_java.jar:../../third_party/cardboard-java/src/CardboardSample/libs/cardboard.jar:../../out/Release/lib.java/content_java.jar:../../out/Release/lib.java/dom_distiller_java.jar:/android/chromium/src/third_party/android_tools/sdk//extras/google/gcm/gcm-client/dist/gcm.jar:../../out/Release/lib.java/cacheinvalidation_proto_java.jar:../../out/Release/lib.java/cacheinvalidation_javalib.jar:../../out/Release/lib.java/sync_java.jar:../../out/Release/lib.java/gcm_driver_java.jar:../../out/Release/lib.java/invalidation_proto_java.jar:../../out/Release/lib.java/invalidation_java.jar:../../out/Release/lib.java/navigation_interception_java.jar:../../out/Release/lib.java/precache_java.jar:../../out/Release/lib.java/safe_json_java.jar:../../out/Release/lib.java/service_tab_launcher_java.jar:../../out/Release/lib.java/signin_core_browser_java.jar:../../out/Release/lib.java/variations_java.jar:../../out/Release/lib.java/web_contents_delegate_android_java.jar:../../out/Release/lib.java/web_restriction_java.jar:../../out/Release/lib.java/printing_java.jar:../../out/Release/lib.java/android_data_chart_java.jar:../../out/Release/lib.java/android_media_java.jar:../../out/Release/lib.java/android_swipe_refresh_java.jar:/android/chromium/src/third_party/android_tools/sdk//extras/android/support/v7/recyclerview/libs/android-support-v7-recyclerview.jar:../../out/Release/lib.java/android_support_v7_recyclerview_javalib.jar:../../out/Release/lib.java/gif_player_java.jar:../../out/Release/lib.java/policy_java.jar:../../out/Release/lib.java/chrome_java.jar:../../out/Release/lib.java/clank_java.jar:../../out/Release/lib.java/android_webview_google_java.jar:../../out/Release/lib.java/external_video_surface_java.jar:../../out/Release/lib.java/android_webview_java.jar:../../out/Release/lib.java/system_webview_glue_java.jar:../../out/Release/lib.java/chromium_apk_monochrome_apk.jar > -include ../../android_webview/apk/java/proguard.flags -include > ../../chrome/android/java/proguard.flags -include > ../../clank/java/proguard.flags -include > ../../out/Release/monochrome_apk/proguard.txt -outjars > ../../out/Release/monochrome_apk/obfuscated.jar -dump > ../../out/Release/monochrome_apk/obfuscated.jar.dump -printseeds > ../../out/Release/monochrome_apk/obfuscated.jar.seeds -printusage > ../../out/Release/monochrome_apk/obfuscated.jar.usage -printmapping > ../../out/Release/monochrome_apk/obfuscated.jar.mapping ) > Warning: org.chromium.chrome.browser.ChromeApplication: can't find referenced > method 'ActivityWindowAndroid(android.app.Activity)' in class > org.chromium.ui.base.ActivityWindowAndroid > Warning: org.chromium.chrome.browser.ChromeWindow: can't find referenced method > 'ActivityWindowAndroid(android.app.Activity)' in class > org.chromium.ui.base.ActivityWindowAndroid > Warning: there were 2 unresolved references to program class members. > Your input classes appear to be inconsistent. > You may need to recompile them and try again. > Alternatively, you may have to specify the option > '-dontskipnonpubliclibraryclassmembers'. > Error: Please correct the above warnings first. > """
On 2016/01/10 23:30:09, jbudorick wrote: > On 2016/01/10 22:21:31, boliu wrote: > > Talked to Yusuf offline, then tested more.. Any white-space change to > > ChromeWindow.java would have fixed the the build error, although simply > touching > > the file isn't enough. By extension, SelectFileDialogTest.java probably also > > needs such a change too. Fwiw land mine would have worked too. > > > > Fwiw the javac.py command for chrome_java is issued, so the gyp dependencies > are > > correct. I *think* the actual javac command never happans, this is a bug in > > javac.py. Haven't worked out exactly what yet. > > agrieve, any thoughts on what might have happened here? Hmm, ui_java.jar.TOC only includes public classes, but not public methods in those classes. And I think only the TOC file is checked to see if any dependent jars needs to be rebuilt, not the actual content of ui_java.jar. So I *think* correct solution is list public and protected methods in the TOC as well. Not that I know any of this code that well..
On 2016/01/11 01:18:15, boliu wrote: > On 2016/01/10 23:30:09, jbudorick wrote: > > On 2016/01/10 22:21:31, boliu wrote: > > > Talked to Yusuf offline, then tested more.. Any white-space change to > > > ChromeWindow.java would have fixed the the build error, although simply > > touching > > > the file isn't enough. By extension, SelectFileDialogTest.java probably also > > > needs such a change too. Fwiw land mine would have worked too. > > > > > > Fwiw the javac.py command for chrome_java is issued, so the gyp dependencies > > are > > > correct. I *think* the actual javac command never happans, this is a bug in > > > javac.py. Haven't worked out exactly what yet. > > > > agrieve, any thoughts on what might have happened here? > > Hmm, ui_java.jar.TOC only includes public classes, but not public methods in > those classes. And I think only the TOC file is checked to see if any dependent > jars needs to be rebuilt, not the actual content of ui_java.jar. So I *think* > correct solution is list public and protected methods in the TOC as well. Not > that I know any of this code that well.. Yeah, regex in jar_toc.ExtractToc is definitely broken: https://code.google.com/p/chromium/codesearch#chromium/src/build/android/gyp/... This regex that says "This includes all class/function/member signatures." doesn't match anything anymore. So this has been broken for awhile now, maybe since last time we bumped the java toolchain? TOC bug should be fixed, but I honestly I still vote for landmine with this CL, since it needs to be merged back to m48
On 2016/01/11 01:49:13, boliu wrote: > On 2016/01/11 01:18:15, boliu wrote: > > On 2016/01/10 23:30:09, jbudorick wrote: > > > On 2016/01/10 22:21:31, boliu wrote: > > > > Talked to Yusuf offline, then tested more.. Any white-space change to > > > > ChromeWindow.java would have fixed the the build error, although simply > > > touching > > > > the file isn't enough. By extension, SelectFileDialogTest.java probably > also > > > > needs such a change too. Fwiw land mine would have worked too. > > > > > > > > Fwiw the javac.py command for chrome_java is issued, so the gyp > dependencies > > > are > > > > correct. I *think* the actual javac command never happans, this is a bug > in > > > > javac.py. Haven't worked out exactly what yet. > > > > > > agrieve, any thoughts on what might have happened here? > > > > Hmm, ui_java.jar.TOC only includes public classes, but not public methods in > > those classes. And I think only the TOC file is checked to see if any > dependent > > jars needs to be rebuilt, not the actual content of ui_java.jar. So I *think* > > correct solution is list public and protected methods in the TOC as well. Not > > that I know any of this code that well.. > > Yeah, regex in jar_toc.ExtractToc is definitely broken: > https://code.google.com/p/chromium/codesearch#chromium/src/build/android/gyp/... > This regex that says "This includes all class/function/member signatures." > doesn't match anything anymore. So this has been broken for awhile now, maybe > since last time we bumped the java toolchain? > > TOC bug should be fixed, but I honestly I still vote for landmine with this CL, > since it needs to be merged back to m48 TOC fix: https://codereview.chromium.org/1573023002/
On 2016/01/11 01:49:13, boliu wrote: > On 2016/01/11 01:18:15, boliu wrote: > > On 2016/01/10 23:30:09, jbudorick wrote: > > > On 2016/01/10 22:21:31, boliu wrote: > > > > Talked to Yusuf offline, then tested more.. Any white-space change to > > > > ChromeWindow.java would have fixed the the build error, although simply > > > touching > > > > the file isn't enough. By extension, SelectFileDialogTest.java probably > also > > > > needs such a change too. Fwiw land mine would have worked too. > > > > > > > > Fwiw the javac.py command for chrome_java is issued, so the gyp > dependencies > > > are > > > > correct. I *think* the actual javac command never happans, this is a bug > in > > > > javac.py. Haven't worked out exactly what yet. > > > > > > agrieve, any thoughts on what might have happened here? > > > > Hmm, ui_java.jar.TOC only includes public classes, but not public methods in > > those classes. And I think only the TOC file is checked to see if any > dependent > > jars needs to be rebuilt, not the actual content of ui_java.jar. So I *think* > > correct solution is list public and protected methods in the TOC as well. Not > > that I know any of this code that well.. > > Yeah, regex in jar_toc.ExtractToc is definitely broken: > https://code.google.com/p/chromium/codesearch#chromium/src/build/android/gyp/... > This regex that says "This includes all class/function/member signatures." > doesn't match anything anymore. So this has been broken for awhile now, maybe > since last time we bumped the java toolchain? > > TOC bug should be fixed, but I honestly I still vote for landmine with this CL, > since it needs to be merged back to m48 I'd rather backport the TOC fix along with this CL.
lgtm
The CQ bit was checked by yusufo@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from boliu@chromium.org, tedchoc@chromium.org Link to the patchset: https://codereview.chromium.org/1569643002/#ps60001 (title: "Revert the changes in ChromeWindow as they are not needed anymore")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1569643002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1569643002/60001
Message was sent while issue was closed.
Description was changed from ========== Reland Pull the Activity context from WindowAndroid if possible WebView uses ContentViewCore's context for displaying dialogs and wraps it at construction time. This change makes sure that if there is an activity that can be reached through WindowAndroid, we use that as the context inside ColorChooser. Also make sure if there is no activity, we dont attempt to create a color chooser and try to show it. Update ActivityWindowAndroid to take a Context type so that in Android WebView, the activity context can remain wrapped. BUG=550410, 570429 ========== to ========== Reland Pull the Activity context from WindowAndroid if possible WebView uses ContentViewCore's context for displaying dialogs and wraps it at construction time. This change makes sure that if there is an activity that can be reached through WindowAndroid, we use that as the context inside ColorChooser. Also make sure if there is no activity, we dont attempt to create a color chooser and try to show it. Update ActivityWindowAndroid to take a Context type so that in Android WebView, the activity context can remain wrapped. BUG=550410, 570429 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Reland Pull the Activity context from WindowAndroid if possible WebView uses ContentViewCore's context for displaying dialogs and wraps it at construction time. This change makes sure that if there is an activity that can be reached through WindowAndroid, we use that as the context inside ColorChooser. Also make sure if there is no activity, we dont attempt to create a color chooser and try to show it. Update ActivityWindowAndroid to take a Context type so that in Android WebView, the activity context can remain wrapped. BUG=550410, 570429 ========== to ========== Reland Pull the Activity context from WindowAndroid if possible WebView uses ContentViewCore's context for displaying dialogs and wraps it at construction time. This change makes sure that if there is an activity that can be reached through WindowAndroid, we use that as the context inside ColorChooser. Also make sure if there is no activity, we dont attempt to create a color chooser and try to show it. Update ActivityWindowAndroid to take a Context type so that in Android WebView, the activity context can remain wrapped. BUG=550410, 570429 Committed: https://crrev.com/235c2e05a35759c50b0f106ab1f7aec3142eebb1 Cr-Commit-Position: refs/heads/master@{#368702} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/235c2e05a35759c50b0f106ab1f7aec3142eebb1 Cr-Commit-Position: refs/heads/master@{#368702} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
