210296 2020-04-09 12:39:49 -0700 [Cocoa] The function WebCore::systemHasBattery() should cache the result. 2020-04-09 14:04:53 -0700 1 1 1 Unclassified WebKit WebKit Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 pvollan pvollan bfulgham darin webkit-bug-importer oldest_to_newest 1639787 0 pvollan 2020-04-09 12:39:49 -0700 The function WebCore::systemHasBattery() should cache the result, since the return value of this function will be the same on a specific device. 1639789 1 pvollan 2020-04-09 12:40:18 -0700 rdar://problem/61331536 1639791 2 395991 pvollan 2020-04-09 12:42:46 -0700 Created attachment 395991 Patch 1639796 3 395991 darin 2020-04-09 12:49:47 -0700 Comment on attachment 395991 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=395991&action=review > Source/WebCore/platform/cocoa/SystemBattery.mm:45 > + static bool hasBattery = [] { Very confusing that we have two different global booleans of different types and scopes, but the same names. I suggest not defining a second boolean here, and just assigning a value to the first one. > Source/WebCore/platform/cocoa/SystemBattery.mm:61 > + return hasBattery; This would then be: return *hasBattery; Or you could write the function as: if (!hasBattery) { hasBattery = [] { <....> }(); } return *hasBattery; 1639800 4 395992 pvollan 2020-04-09 12:58:41 -0700 Created attachment 395992 Patch 1639801 5 pvollan 2020-04-09 12:59:22 -0700 (In reply to Darin Adler from comment #3) > Comment on attachment 395991 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=395991&action=review > > > Source/WebCore/platform/cocoa/SystemBattery.mm:45 > > + static bool hasBattery = [] { > > Very confusing that we have two different global booleans of different types > and scopes, but the same names. > > I suggest not defining a second boolean here, and just assigning a value to > the first one. > > > Source/WebCore/platform/cocoa/SystemBattery.mm:61 > > + return hasBattery; > > This would then be: > > return *hasBattery; > > Or you could write the function as: > > if (!hasBattery) { > hasBattery = [] { > <....> > }(); > } > return *hasBattery; Very good point, thanks for reviewing! 1639859 6 ews-feeder 2020-04-09 14:04:52 -0700 Committed r259827: <https://trac.webkit.org/changeset/259827> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395992. 395991 2020-04-09 12:42:46 -0700 2020-04-09 12:58:40 -0700 Patch bug-210296-20200409124245.patch text/plain 2847 pvollan SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI1OTgxNCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDIwLTA0LTA5ICBQZXIgQXJu ZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbQ29jb2FdIFRoZSBmdW5j dGlvbiBXZWJDb3JlOjpzeXN0ZW1IYXNCYXR0ZXJ5KCkgc2hvdWxkIGNhY2hlIHRoZSByZXN1bHQu CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTAyOTYK KyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzYxMzMxNTM2PgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBmdW5jdGlvbiBXZWJDb3JlOjpzeXN0ZW1I YXNCYXR0ZXJ5KCkgc2hvdWxkIGNhY2hlIHRoZSByZXN1bHQsIHNpbmNlIHRoZSByZXR1cm4gdmFs dWUgb2YgdGhpcyBmdW5jdGlvbgorICAgICAgICB3aWxsIGJlIHRoZSBzYW1lIG9uIGEgc3BlY2lm aWMgZGV2aWNlLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cywgc2luY2UgdGhlcmUgaXMgbm8gY2hh bmdlIGluIGJlaGF2aW9yLgorCisgICAgICAgICogcGxhdGZvcm0vY29jb2EvU3lzdGVtQmF0dGVy eS5tbToKKyAgICAgICAgKFdlYkNvcmU6OnN5c3RlbUhhc0JhdHRlcnkpOgorCiAyMDIwLTA0LTA5 ICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KIAogICAgICAgICBSZW1vdmVkIHVu dXNlZCBmaWVsZHMgYW5kIGZ1bmN0aW9ucyBmcm9tIFN0eWxlUHJvcGVydHlTaG9ydGhhbmQKSW5k ZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2NvY29hL1N5c3RlbUJhdHRlcnkubW0KPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY29jb2EvU3lzdGVtQmF0dGVyeS5tbQko cmV2aXNpb24gMjU5NzYyKQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY29jb2EvU3lzdGVt QmF0dGVyeS5tbQkod29ya2luZyBjb3B5KQpAQCAtNDIsMTkgKzQyLDIzIEBAIGJvb2wgc3lzdGVt SGFzQmF0dGVyeSgpCiAgICAgaWYgKGhhc0JhdHRlcnkuaGFzVmFsdWUoKSkKICAgICAgICAgcmV0 dXJuICpoYXNCYXR0ZXJ5OwogCi0gICAgUmV0YWluUHRyPENGVHlwZVJlZj4gcG93ZXJTb3VyY2Vz SW5mbyA9IGFkb3B0Q0YoSU9QU0NvcHlQb3dlclNvdXJjZXNJbmZvKCkpOwotICAgIGlmICghcG93 ZXJTb3VyY2VzSW5mbykKKyAgICBzdGF0aWMgYm9vbCBoYXNCYXR0ZXJ5ID0gW10geworICAgICAg ICBSZXRhaW5QdHI8Q0ZUeXBlUmVmPiBwb3dlclNvdXJjZXNJbmZvID0gYWRvcHRDRihJT1BTQ29w eVBvd2VyU291cmNlc0luZm8oKSk7CisgICAgICAgIGlmICghcG93ZXJTb3VyY2VzSW5mbykKKyAg ICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgUmV0YWluUHRyPENGQXJyYXlSZWY+IHBv d2VyU291cmNlc0xpc3QgPSBhZG9wdENGKElPUFNDb3B5UG93ZXJTb3VyY2VzTGlzdChwb3dlclNv dXJjZXNJbmZvLmdldCgpKSk7CisgICAgICAgIGlmICghcG93ZXJTb3VyY2VzTGlzdCkKKyAgICAg ICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgZm9yIChDRkluZGV4IGkgPSAwLCBjb3VudCA9 IENGQXJyYXlHZXRDb3VudChwb3dlclNvdXJjZXNMaXN0LmdldCgpKTsgaSA8IGNvdW50OyArK2kp IHsKKyAgICAgICAgICAgIENGRGljdGlvbmFyeVJlZiBkZXNjcmlwdGlvbiA9IElPUFNHZXRQb3dl clNvdXJjZURlc2NyaXB0aW9uKHBvd2VyU291cmNlc0luZm8uZ2V0KCksIENGQXJyYXlHZXRWYWx1 ZUF0SW5kZXgocG93ZXJTb3VyY2VzTGlzdC5nZXQoKSwgaSkpOworICAgICAgICAgICAgQ0ZUeXBl UmVmIHZhbHVlID0gQ0ZEaWN0aW9uYXJ5R2V0VmFsdWUoZGVzY3JpcHRpb24sIENGU1RSKGtJT1BT VHlwZUtleSkpOworICAgICAgICAgICAgaWYgKCF2YWx1ZSB8fCBDRkVxdWFsKHZhbHVlLCBDRlNU UihrSU9QU0ludGVybmFsQmF0dGVyeVR5cGUpKSkKKyAgICAgICAgICAgICAgICByZXR1cm4gdHJ1 ZTsKKyAgICAgICAgfQogICAgICAgICByZXR1cm4gZmFsc2U7Ci0gICAgUmV0YWluUHRyPENGQXJy YXlSZWY+IHBvd2VyU291cmNlc0xpc3QgPSBhZG9wdENGKElPUFNDb3B5UG93ZXJTb3VyY2VzTGlz dChwb3dlclNvdXJjZXNJbmZvLmdldCgpKSk7Ci0gICAgaWYgKCFwb3dlclNvdXJjZXNMaXN0KQot ICAgICAgICByZXR1cm4gZmFsc2U7Ci0gICAgZm9yIChDRkluZGV4IGkgPSAwLCBjb3VudCA9IENG QXJyYXlHZXRDb3VudChwb3dlclNvdXJjZXNMaXN0LmdldCgpKTsgaSA8IGNvdW50OyArK2kpIHsK LSAgICAgICAgQ0ZEaWN0aW9uYXJ5UmVmIGRlc2NyaXB0aW9uID0gSU9QU0dldFBvd2VyU291cmNl RGVzY3JpcHRpb24ocG93ZXJTb3VyY2VzSW5mby5nZXQoKSwgQ0ZBcnJheUdldFZhbHVlQXRJbmRl eChwb3dlclNvdXJjZXNMaXN0LmdldCgpLCBpKSk7Ci0gICAgICAgIENGVHlwZVJlZiB2YWx1ZSA9 IENGRGljdGlvbmFyeUdldFZhbHVlKGRlc2NyaXB0aW9uLCBDRlNUUihrSU9QU1R5cGVLZXkpKTsK LSAgICAgICAgaWYgKCF2YWx1ZSB8fCBDRkVxdWFsKHZhbHVlLCBDRlNUUihrSU9QU0ludGVybmFs QmF0dGVyeVR5cGUpKSkKLSAgICAgICAgICAgIHJldHVybiB0cnVlOwotICAgIH0KLSAgICByZXR1 cm4gZmFsc2U7CisgICAgfSgpOworCisgICAgcmV0dXJuIGhhc0JhdHRlcnk7CiB9CiAKIH0K 395992 2020-04-09 12:58:41 -0700 2020-04-09 14:04:53 -0700 Patch bug-210296-20200409125840.patch text/plain 2999 pvollan SW5kZXg6IFNvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2UvV2Vi Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDI1OTgxNCkKKysrIFNvdXJjZS9XZWJDb3JlL0NoYW5n ZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDIwLTA0LTA5ICBQZXIgQXJu ZSBWb2xsYW4gIDxwdm9sbGFuQGFwcGxlLmNvbT4KKworICAgICAgICBbQ29jb2FdIFRoZSBmdW5j dGlvbiBXZWJDb3JlOjpzeXN0ZW1IYXNCYXR0ZXJ5KCkgc2hvdWxkIGNhY2hlIHRoZSByZXN1bHQu CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMTAyOTYK KyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzYxMzMxNTM2PgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBmdW5jdGlvbiBXZWJDb3JlOjpzeXN0ZW1I YXNCYXR0ZXJ5KCkgc2hvdWxkIGNhY2hlIHRoZSByZXN1bHQsIHNpbmNlIHRoZSByZXR1cm4gdmFs dWUgb2YgdGhpcyBmdW5jdGlvbgorICAgICAgICB3aWxsIGJlIHRoZSBzYW1lIG9uIGEgc3BlY2lm aWMgZGV2aWNlLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cywgc2luY2UgdGhlcmUgaXMgbm8gY2hh bmdlIGluIGJlaGF2aW9yLgorCisgICAgICAgICogcGxhdGZvcm0vY29jb2EvU3lzdGVtQmF0dGVy eS5tbToKKyAgICAgICAgKFdlYkNvcmU6OnN5c3RlbUhhc0JhdHRlcnkpOgorCiAyMDIwLTA0LTA5 ICBBbnR0aSBLb2l2aXN0byAgPGFudHRpQGFwcGxlLmNvbT4KIAogICAgICAgICBSZW1vdmVkIHVu dXNlZCBmaWVsZHMgYW5kIGZ1bmN0aW9ucyBmcm9tIFN0eWxlUHJvcGVydHlTaG9ydGhhbmQKSW5k ZXg6IFNvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2NvY29hL1N5c3RlbUJhdHRlcnkubW0KPT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PQotLS0gU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY29jb2EvU3lzdGVtQmF0dGVyeS5tbQko cmV2aXNpb24gMjU5NzYyKQorKysgU291cmNlL1dlYkNvcmUvcGxhdGZvcm0vY29jb2EvU3lzdGVt QmF0dGVyeS5tbQkod29ya2luZyBjb3B5KQpAQCAtMzksMjIgKzM5LDI1IEBAIHZvaWQgc2V0U3lz dGVtSGFzQmF0dGVyeShib29sIGJhdHRlcnkpCiAKIGJvb2wgc3lzdGVtSGFzQmF0dGVyeSgpCiB7 Ci0gICAgaWYgKGhhc0JhdHRlcnkuaGFzVmFsdWUoKSkKLSAgICAgICAgcmV0dXJuICpoYXNCYXR0 ZXJ5OwotCi0gICAgUmV0YWluUHRyPENGVHlwZVJlZj4gcG93ZXJTb3VyY2VzSW5mbyA9IGFkb3B0 Q0YoSU9QU0NvcHlQb3dlclNvdXJjZXNJbmZvKCkpOwotICAgIGlmICghcG93ZXJTb3VyY2VzSW5m bykKLSAgICAgICAgcmV0dXJuIGZhbHNlOwotICAgIFJldGFpblB0cjxDRkFycmF5UmVmPiBwb3dl clNvdXJjZXNMaXN0ID0gYWRvcHRDRihJT1BTQ29weVBvd2VyU291cmNlc0xpc3QocG93ZXJTb3Vy Y2VzSW5mby5nZXQoKSkpOwotICAgIGlmICghcG93ZXJTb3VyY2VzTGlzdCkKLSAgICAgICAgcmV0 dXJuIGZhbHNlOwotICAgIGZvciAoQ0ZJbmRleCBpID0gMCwgY291bnQgPSBDRkFycmF5R2V0Q291 bnQocG93ZXJTb3VyY2VzTGlzdC5nZXQoKSk7IGkgPCBjb3VudDsgKytpKSB7Ci0gICAgICAgIENG RGljdGlvbmFyeVJlZiBkZXNjcmlwdGlvbiA9IElPUFNHZXRQb3dlclNvdXJjZURlc2NyaXB0aW9u KHBvd2VyU291cmNlc0luZm8uZ2V0KCksIENGQXJyYXlHZXRWYWx1ZUF0SW5kZXgocG93ZXJTb3Vy Y2VzTGlzdC5nZXQoKSwgaSkpOwotICAgICAgICBDRlR5cGVSZWYgdmFsdWUgPSBDRkRpY3Rpb25h cnlHZXRWYWx1ZShkZXNjcmlwdGlvbiwgQ0ZTVFIoa0lPUFNUeXBlS2V5KSk7Ci0gICAgICAgIGlm ICghdmFsdWUgfHwgQ0ZFcXVhbCh2YWx1ZSwgQ0ZTVFIoa0lPUFNJbnRlcm5hbEJhdHRlcnlUeXBl KSkpCi0gICAgICAgICAgICByZXR1cm4gdHJ1ZTsKKyAgICBpZiAoIWhhc0JhdHRlcnkuaGFzVmFs dWUoKSkgeworICAgICAgICBoYXNCYXR0ZXJ5ID0gW10geworICAgICAgICAgICAgUmV0YWluUHRy PENGVHlwZVJlZj4gcG93ZXJTb3VyY2VzSW5mbyA9IGFkb3B0Q0YoSU9QU0NvcHlQb3dlclNvdXJj ZXNJbmZvKCkpOworICAgICAgICAgICAgaWYgKCFwb3dlclNvdXJjZXNJbmZvKQorICAgICAgICAg ICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgICAgIFJldGFpblB0cjxDRkFycmF5UmVmPiBw b3dlclNvdXJjZXNMaXN0ID0gYWRvcHRDRihJT1BTQ29weVBvd2VyU291cmNlc0xpc3QocG93ZXJT b3VyY2VzSW5mby5nZXQoKSkpOworICAgICAgICAgICAgaWYgKCFwb3dlclNvdXJjZXNMaXN0KQor ICAgICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKKyAgICAgICAgICAgIGZvciAoQ0ZJbmRleCBp ID0gMCwgY291bnQgPSBDRkFycmF5R2V0Q291bnQocG93ZXJTb3VyY2VzTGlzdC5nZXQoKSk7IGkg PCBjb3VudDsgKytpKSB7CisgICAgICAgICAgICAgICAgQ0ZEaWN0aW9uYXJ5UmVmIGRlc2NyaXB0 aW9uID0gSU9QU0dldFBvd2VyU291cmNlRGVzY3JpcHRpb24ocG93ZXJTb3VyY2VzSW5mby5nZXQo KSwgQ0ZBcnJheUdldFZhbHVlQXRJbmRleChwb3dlclNvdXJjZXNMaXN0LmdldCgpLCBpKSk7Cisg ICAgICAgICAgICAgICAgQ0ZUeXBlUmVmIHZhbHVlID0gQ0ZEaWN0aW9uYXJ5R2V0VmFsdWUoZGVz Y3JpcHRpb24sIENGU1RSKGtJT1BTVHlwZUtleSkpOworICAgICAgICAgICAgICAgIGlmICghdmFs dWUgfHwgQ0ZFcXVhbCh2YWx1ZSwgQ0ZTVFIoa0lPUFNJbnRlcm5hbEJhdHRlcnlUeXBlKSkpCisg ICAgICAgICAgICAgICAgICAgIHJldHVybiB0cnVlOworICAgICAgICAgICAgfQorICAgICAgICAg ICAgcmV0dXJuIGZhbHNlOworICAgICAgICB9KCk7CiAgICAgfQotICAgIHJldHVybiBmYWxzZTsK KworICAgIHJldHVybiAqaGFzQmF0dGVyeTsKIH0KIAogfQo=