Enable DLL preloading when MozillaMaintenance service disabled prefetch

VERIFIED FIXED in mozilla15

Status

()

Core
Widget: Win32
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: bbondy, Assigned: bbondy)

Tracking

unspecified
mozilla15
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [qa?])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

6 years ago
We currently only do DLL preloading when we think that the Windows prefetch isn't used.  We should also do DLL preloading when the maintenance service has performed a successful clear prefetch operation.
(Assignee)

Comment 1

6 years ago
Created attachment 597866 [details] [diff] [review]
Patch v1.
Attachment #597866 - Flags: review?(jmathies)
(Assignee)

Comment 2

6 years ago
Alfred Kayser 2012-02-16 23:59:42 PST

I would move the LOG statement of the failing WritePrefetchClearedReg function inside that function, just as the other LOG statement is also there.
+  if (retCode != ERROR_SUCCESS) {
+    LOG(("Could not open key. (%d)\n", retCode));
+  }
And, add a 'return false' here after the LOG.
(Assignee)

Comment 3

6 years ago
Created attachment 598233 [details] [diff] [review]
Patch v2.

Implemented review comments and rebased to m-c tip.
Attachment #597866 - Attachment is obsolete: true
Attachment #598233 - Flags: review?(jmathies)
Attachment #597866 - Flags: review?(jmathies)

Comment 4

6 years ago
Comment on attachment 598233 [details] [diff] [review]
Patch v2.

Review of attachment 598233 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/app/nsBrowserApp.cpp
@@ +251,5 @@
>    // in the program. Luckily 1 coincides with when prefetch is
>    // enabled. If Windows prefetch didn't happen we can do our own
>    // faster dll preloading.
> +  // The MozillaMaintenance service issues a command to disable the
> +  // pretch by replacing all found .pf files with 0 byte read only

nit - 'pretch'

::: toolkit/components/maintenanceservice/prefetch.cpp
@@ +54,5 @@
> +  LONG retCode = RegOpenKeyExW(HKEY_LOCAL_MACHINE, 
> +                               BASE_SERVICE_REG_KEY, 0,
> +                               KEY_WRITE | KEY_WOW64_64KEY, &baseKeyRaw);
> +  if (retCode != ERROR_SUCCESS) {
> +    LOG(("Could not open key. (%d)\n", retCode));

does this LOG macro do file name / line number logging? If not you might want to make that msg more specific.
Attachment #598233 - Flags: review?(jmathies) → review+
(Assignee)

Comment 5

6 years ago
Created attachment 599602 [details] [diff] [review]
Patch v3.

Implemented nits, carrying forward r+.
Attachment #598233 - Attachment is obsolete: true
Attachment #599602 - Flags: superreview?(robert.bugzilla)
Attachment #599602 - Flags: review+
(Assignee)

Comment 6

6 years ago
Comment on attachment 599602 [details] [diff] [review]
Patch v3.

Actually I don't think this one needs a super-review so clearing that flag.
Attachment #599602 - Flags: superreview?(robert.bugzilla)
(Assignee)

Comment 7

5 years ago
Created attachment 623704 [details] [diff] [review]
Patch v4

Rebased to tip
Attachment #599602 - Attachment is obsolete: true
Attachment #623704 - Flags: review+

Updated

5 years ago
No longer depends on: 692255

Updated

5 years ago
Depends on: 692255
(Assignee)

Updated

5 years ago
Depends on: 758206
(Assignee)

Comment 8

5 years ago
http://hg.mozilla.org/mozilla-central/rev/21ae85ff4286
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: mozilla13 → mozilla15

Updated

5 years ago
Whiteboard: [qa+]
Is there anything to verify here from an end-user perspective?
Whiteboard: [qa+] → [qa?]
(Assignee)

Comment 10

5 years ago
No this was verified by seeing telemetry data.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.