62118 2011-06-06 03:47:48 -0700 Use of ENABLE(INSPECTOR) is inconsistent 2011-06-06 07:15:50 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 steveblock webkit-unassigned commit-queue mnaganov pfeldman steveblock yurys oldest_to_newest 415769 0 steveblock 2011-06-06 03:47:48 -0700 Android does not enable INSPECTOR and this results in build errors after http://trac.webkit.org/changeset/81265 http://trac.webkit.org/changeset/81265 added a call to ScriptProfiler::initialize() from V8DOMWindowShell::initContextIfNeeded() (see also Bug 62014). ScriptProfiler requires ScriptProfile, which in turn depends upon InspectorObject. InspectorObject is provided in InspectorValues.h, but the contents are guarded with ENABLE(INSPECTOR), which Android does not enable. We could simply guard the call to ScriptProfiler::initialize() from V8DOMWindowShell::initContextIfNeeded() with ENABLE(INSPECTOR), or fix what seems to be a more general problem with the use of ENABLE(INSPECTOR). 415771 1 96074 steveblock 2011-06-06 03:53:19 -0700 Created attachment 96074 Patch 415772 2 mnaganov 2011-06-06 04:07:32 -0700 Yes, I think, it's acceptable. Yury, can you please review this? 415775 3 96074 yurys 2011-06-06 04:34:46 -0700 Comment on attachment 96074 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96074&action=review > Source/WebCore/bindings/v8/V8DOMWindowShell.cpp:301 > +#if ENABLE(INSPECTOR) If ScriptProfiler call is guarded with ENABLE(INSPECTOR) here then corresponding part of ScriptProfiler implementation should also be guarded, r- for this. Also, I'd rather keep all guards in ScriptProfiler.cpp 415778 4 steveblock 2011-06-06 04:57:12 -0700 > Also, I'd rather keep all guards in ScriptProfiler.cpp So we should guard the contents of ScriptProfiler::initialize()? The problem is that I can't compile ScriptProfiler without also compiling ScriptProfile, but this requires InspectorObject, which is guarded by ENABLE(INSPECTOR). Should all of other methods of ScriptProfiler be guarded by EANBLE(INSPECTOR)? 415782 5 yurys 2011-06-06 05:07:56 -0700 (In reply to comment #4) > > Also, I'd rather keep all guards in ScriptProfiler.cpp > So we should guard the contents of ScriptProfiler::initialize()? > I'd do so, but I don't mind if you leave the guard at the call site as well. We regularly place guards at call sites and around whole implementation of the class but I think it won't work in this case as part of ScriptProfiler should be available when inspector is disabled. > The problem is that I can't compile ScriptProfiler without also compiling ScriptProfile, but this requires InspectorObject, which is guarded by ENABLE(INSPECTOR). Should all of other methods of ScriptProfiler be guarded by EANBLE(INSPECTOR)? Yes, all methods that are only used by inspector should be guarded by ENABLE(INSPECTOR), this should make ScriptProfiler compile even when inspector is disabled. 415793 6 96079 steveblock 2011-06-06 05:47:18 -0700 Created attachment 96079 Patch 415794 7 96080 steveblock 2011-06-06 05:48:31 -0700 Created attachment 96080 Patch 415816 8 commit-queue 2011-06-06 07:14:05 -0700 The commit-queue encountered the following flaky tests while processing attachment 96080: inspector/debugger/source-frame.html bug 57399 (author: podivilov@chromium.org) The commit-queue is continuing to process your patch. 415817 9 96080 commit-queue 2011-06-06 07:15:45 -0700 Comment on attachment 96080 Patch Clearing flags on attachment: 96080 Committed r88166: <http://trac.webkit.org/changeset/88166> 415818 10 commit-queue 2011-06-06 07:15:50 -0700 All reviewed patches have been landed. Closing bug. 96074 2011-06-06 03:53:19 -0700 2011-06-06 05:47:13 -0700 Patch bug-62118-20110606115317.patch text/plain 1268 steveblock SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg4MTUxKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTggQEAKKzIwMTEtMDYtMDYgIFN0ZXZlIEJs b2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFVzZSBvZiBFTkFCTEUoSU5TUEVDVE9SKSBpcyBpbmNvbnNp c3RlbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYy MTE4CisKKyAgICAgICAgR3VhcmRzIGNhbGwgc2l0ZSBvZiBTY3JpcHRQcm9maWxlcjo6aW5pdGlh bGl6ZSgpIHdpdGgKKyAgICAgICAgRU5BQkxFKElOU1BFQ1RPUikuCisKKyAgICAgICAgTm8gbmV3 IHRlc3RzLCBidWlsZCBmaXggb25seS4KKworICAgICAgICAqIGJpbmRpbmdzL3Y4L1Y4RE9NV2lu ZG93U2hlbGwuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VjhET01XaW5kb3dTaGVsbDo6aW5pdENv bnRleHRJZk5lZWRlZCk6CisKIDIwMTEtMDYtMDYgIEFsZXhhbmRydSBDaGljdWxpdGEgIDxhY2hp Y3VAYWRvYmUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEtlbnQgVGFtdXJhLgpJbmRleDog U291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvVjhET01XaW5kb3dTaGVsbC5jcHAKPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQotLS0gU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvVjhET01XaW5kb3dTaGVsbC5jcHAJKHJl dmlzaW9uIDg4MTUxKQorKysgU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvVjhET01XaW5kb3dT aGVsbC5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTI5OCw4ICsyOTgsMTAgQEAgYm9vbCBWOERPTVdp bmRvd1NoZWxsOjppbml0Q29udGV4dElmTmVlZAogCiAgICAgICAgIHY4OjpWODo6U2V0RmFpbGVk QWNjZXNzQ2hlY2tDYWxsYmFja0Z1bmN0aW9uKHJlcG9ydFVuc2FmZUphdmFTY3JpcHRBY2Nlc3Mp OwogCisjaWYgRU5BQkxFKElOU1BFQ1RPUikKICAgICAgICAgU2NyaXB0UHJvZmlsZXI6OmluaXRp YWxpemUoKTsKLSAgICAgICAgCisjZW5kaWYKKwogICAgICAgICBpc1Y4SW5pdGlhbGl6ZWQgPSB0 cnVlOwogICAgIH0KIAo= 96079 2011-06-06 05:47:18 -0700 2011-06-06 05:48:27 -0700 Patch bug-62118-20110606134716.patch text/plain 1489 steveblock SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg4MTYwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDYtMDYgIFN0ZXZlIEJs b2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFVzZSBvZiBFTkFCTEUoSU5TUEVDVE9SKSBpcyBpbmNvbnNp c3RlbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYy MTE4CisKKyAgICAgICAgQ29ycmVjdGx5IGd1YXJkIGltcGxlbWVudGF0aW9uIG9mIFNjcmlwdFBy b2ZpbGVyLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cy4gKE9PUFMhKQorCisgICAgICAgICogYmlu ZGluZ3MvdjgvU2NyaXB0UHJvZmlsZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6U2NyaXB0UHJv ZmlsZXI6OmluaXRpYWxpemUpOgorCiAyMDExLTA2LTA0ICBEb21pbmljIENvb25leSAgPGRvbWlu aWNjQGNocm9taXVtLm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBEaW1pdHJpIEdsYXprb3Yu CkluZGV4OiBTb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9TY3JpcHRQcm9maWxlci5jcHAKPT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PQotLS0gU291cmNlL1dlYkNvcmUvYmluZGluZ3MvdjgvU2NyaXB0UHJvZmlsZXIuY3Bw CShyZXZpc2lvbiA4MjUwNykKKysrIFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1NjcmlwdFBy b2ZpbGVyLmNwcAkod29ya2luZyBjb3B5KQpAQCAtNDAsNiArNDAsNyBAQAogCiBuYW1lc3BhY2Ug V2ViQ29yZSB7CiAKKyNpZiBFTkFCTEUoSU5TUEVDVE9SKQogdm9pZCBTY3JpcHRQcm9maWxlcjo6 c3RhcnQoU2NyaXB0U3RhdGUqIHN0YXRlLCBjb25zdCBTdHJpbmcmIHRpdGxlKQogewogICAgIHY4 OjpIYW5kbGVTY29wZSBoczsKQEAgLTEwOCwxMCArMTA5LDEzIEBAIHN0YXRpYyB2ODo6UmV0YWlu ZWRPYmplY3RJbmZvKiByZXRhaW5lZEQKICAgICBOb2RlKiBub2RlID0gVjhOb2RlOjp0b05hdGl2 ZSh3cmFwcGVyLkFzPHY4OjpPYmplY3Q+KCkpOwogICAgIHJldHVybiBub2RlID8gbmV3IFJldGFp bmVkRE9NSW5mbyhub2RlKSA6IDA7CiB9CisjZW5kaWYgLy8gRU5BQkxFKElOU1BFQ1RPUikKIAog dm9pZCBTY3JpcHRQcm9maWxlcjo6aW5pdGlhbGl6ZSgpCiB7CisjaWYgRU5BQkxFKElOU1BFQ1RP UikKICAgICB2ODo6SGVhcFByb2ZpbGVyOjpEZWZpbmVXcmFwcGVyQ2xhc3ModjhET01TdWJ0cmVl Q2xhc3NJZCwgJnJldGFpbmVkRE9NSW5mbyk7CisjZW5kaWYgLy8gRU5BQkxFKElOU1BFQ1RPUikK IH0KIAogCg== 96080 2011-06-06 05:48:31 -0700 2011-06-06 07:15:45 -0700 Patch bug-62118-20110606134830.patch text/plain 1497 steveblock SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDg4MTYwKQorKysgU291cmNlL1dlYkNvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMDYtMDYgIFN0ZXZlIEJs b2NrICA8c3RldmVibG9ja0Bnb29nbGUuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFVzZSBvZiBFTkFCTEUoSU5TUEVDVE9SKSBpcyBpbmNvbnNp c3RlbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTYy MTE4CisKKyAgICAgICAgQ29ycmVjdGx5IGd1YXJkIGltcGxlbWVudGF0aW9uIG9mIFNjcmlwdFBy b2ZpbGVyLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cywgYnVpbGQgZml4IG9ubHkuCisKKyAgICAg ICAgKiBiaW5kaW5ncy92OC9TY3JpcHRQcm9maWxlci5jcHA6CisgICAgICAgIChXZWJDb3JlOjpT Y3JpcHRQcm9maWxlcjo6aW5pdGlhbGl6ZSk6CisKIDIwMTEtMDYtMDQgIERvbWluaWMgQ29vbmV5 ICA8ZG9taW5pY2NAY2hyb21pdW0ub3JnPgogCiAgICAgICAgIFJldmlld2VkIGJ5IERpbWl0cmkg R2xhemtvdi4KSW5kZXg6IFNvdXJjZS9XZWJDb3JlL2JpbmRpbmdzL3Y4L1NjcmlwdFByb2ZpbGVy LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy92OC9TY3JpcHRQcm9m aWxlci5jcHAJKHJldmlzaW9uIDgyNTA3KQorKysgU291cmNlL1dlYkNvcmUvYmluZGluZ3Mvdjgv U2NyaXB0UHJvZmlsZXIuY3BwCSh3b3JraW5nIGNvcHkpCkBAIC00MCw2ICs0MCw3IEBACiAKIG5h bWVzcGFjZSBXZWJDb3JlIHsKIAorI2lmIEVOQUJMRShJTlNQRUNUT1IpCiB2b2lkIFNjcmlwdFBy b2ZpbGVyOjpzdGFydChTY3JpcHRTdGF0ZSogc3RhdGUsIGNvbnN0IFN0cmluZyYgdGl0bGUpCiB7 CiAgICAgdjg6OkhhbmRsZVNjb3BlIGhzOwpAQCAtMTA4LDEwICsxMDksMTMgQEAgc3RhdGljIHY4 OjpSZXRhaW5lZE9iamVjdEluZm8qIHJldGFpbmVkRAogICAgIE5vZGUqIG5vZGUgPSBWOE5vZGU6 OnRvTmF0aXZlKHdyYXBwZXIuQXM8djg6Ok9iamVjdD4oKSk7CiAgICAgcmV0dXJuIG5vZGUgPyBu ZXcgUmV0YWluZWRET01JbmZvKG5vZGUpIDogMDsKIH0KKyNlbmRpZiAvLyBFTkFCTEUoSU5TUEVD VE9SKQogCiB2b2lkIFNjcmlwdFByb2ZpbGVyOjppbml0aWFsaXplKCkKIHsKKyNpZiBFTkFCTEUo SU5TUEVDVE9SKQogICAgIHY4OjpIZWFwUHJvZmlsZXI6OkRlZmluZVdyYXBwZXJDbGFzcyh2OERP TVN1YnRyZWVDbGFzc0lkLCAmcmV0YWluZWRET01JbmZvKTsKKyNlbmRpZiAvLyBFTkFCTEUoSU5T UEVDVE9SKQogfQogCiAK